[PATCH net-next] ipv6, token: allow for clearing the current device token

[PATCH net-next] ipv6, token: allow for clearing the current device token

[Daniel Borkmann]

The original tokenized iid support implemented via f53adae4eae5 ("net: ipv6:
add tokenized interface identifier support") didn't allow for clearing a
device token as it was intended that this addressing mode was the only one
active for globally scoped IPv6 addresses. Later we relaxed that restriction
via 617fe29d45bd ("net: ipv6: only invalidate previously tokenized addresses"),
and we should also allow for clearing tokens as there's no good reason why
it shouldn't be allowed.

Fixes: 617fe29d45bd ("net: ipv6: only invalidate previously tokenized addresses")
Reported-by: Robin H. Johnson <robbat2@gentoo.org>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 Dave, I think net-next is fine, but don't have any objections if you rather
 want to put it into net. Thanks!

 net/ipv6/addrconf.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 27aed1a..a6c9927 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -4995,15 +4995,13 @@ static int inet6_set_iftoken(struct inet6_dev *idev, struct in6_addr *token)
 {
 	struct inet6_ifaddr *ifp;
 	struct net_device *dev = idev->dev;
-	bool update_rs = false;
+	bool clear_token, update_rs = false;
 	struct in6_addr ll_addr;
 
 	ASSERT_RTNL();
 
 	if (!token)
 		return -EINVAL;
-	if (ipv6_addr_any(token))
-		return -EINVAL;
 	if (dev->flags & (IFF_LOOPBACK | IFF_NOARP))
 		return -EINVAL;
 	if (!ipv6_accept_ra(idev))
@@ -5018,10 +5016,13 @@ static int inet6_set_iftoken(struct inet6_dev *idev, struct in6_addr *token)
 
 	write_unlock_bh(&idev->lock);
 
+	clear_token = ipv6_addr_any(token);
+	if (clear_token)
+		goto update_lft;
+
 	if (!idev->dead && (idev->if_flags & IF_READY) &&
 	    !ipv6_get_lladdr(dev, &ll_addr, IFA_F_TENTATIVE |
 			     IFA_F_OPTIMISTIC)) {
-
 		/* If we're not ready, then normal ifup will take care
 		 * of this. Otherwise, we need to request our rs here.
 		 */
@@ -5029,6 +5030,7 @@ static int inet6_set_iftoken(struct inet6_dev *idev, struct in6_addr *token)
 		update_rs = true;
 	}
 
+update_lft:
 	write_lock_bh(&idev->lock);
 
 	if (update_rs) {
-- 
1.9.3

Re: [PATCH net-next] ipv6, token: allow for clearing the current device token

[Hannes Frederic Sowa]

On 08.04.2016 15:55, Daniel Borkmann wrote:
> The original tokenized iid support implemented via f53adae4eae5 ("net: ipv6:
> add tokenized interface identifier support") didn't allow for clearing a
> device token as it was intended that this addressing mode was the only one
> active for globally scoped IPv6 addresses. Later we relaxed that restriction
> via 617fe29d45bd ("net: ipv6: only invalidate previously tokenized addresses"),
> and we should also allow for clearing tokens as there's no good reason why
> it shouldn't be allowed.
>
> Fixes: 617fe29d45bd ("net: ipv6: only invalidate previously tokenized addresses")
> Reported-by: Robin H. Johnson <robbat2@gentoo.org>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>

Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>

Thanks,
Hannes

Re: [PATCH net-next] ipv6, token: allow for clearing the current device token

[Bjørn Mork]

Daniel Borkmann <daniel@iogearbox.net> writes:

>  
>  	if (!token)
>  		return -EINVAL;
> -	if (ipv6_addr_any(token))
> -		return -EINVAL;
>  	if (dev->flags & (IFF_LOOPBACK | IFF_NOARP))
>  		return -EINVAL;

Not directly related to the patch in question.  It just made me aware of
this restriction...

I realize that I'm a few years late here, but what's with the IFF_NOARP?
Is that just because we can't do DAD for the token based addresses?  How
is that different from manually configuring the whole address?



Bjørn

Re: [PATCH net-next] ipv6, token: allow for clearing the current device token

[Hannes Frederic Sowa]

On Fri, Apr 8, 2016, at 16:18, Bjørn Mork wrote:
> Daniel Borkmann <daniel@iogearbox.net> writes:
> 
> >  
> >  	if (!token)
> >  		return -EINVAL;
> > -	if (ipv6_addr_any(token))
> > -		return -EINVAL;
> >  	if (dev->flags & (IFF_LOOPBACK | IFF_NOARP))
> >  		return -EINVAL;
> 
> Not directly related to the patch in question.  It just made me aware of
> this restriction...
> 
> I realize that I'm a few years late here, but what's with the IFF_NOARP?
> Is that just because we can't do DAD for the token based addresses?  How
> is that different from manually configuring the whole address?

IFF_NOARP is kind of the equivalent to no neighbor discovery. If you set
a token and never get in a router advertisement you never create a
tokenized ip address, thus the feature is useless.

Bye,
Hannes

Re: [PATCH net-next] ipv6, token: allow for clearing the current device token

[Bjørn Mork]

Hannes Frederic Sowa <hannes@stressinduktion.org> writes:

> On Fri, Apr 8, 2016, at 16:18, Bjørn Mork wrote:
>> Daniel Borkmann <daniel@iogearbox.net> writes:
>> 
>> >  
>> >  	if (!token)
>> >  		return -EINVAL;
>> > -	if (ipv6_addr_any(token))
>> > -		return -EINVAL;
>> >  	if (dev->flags & (IFF_LOOPBACK | IFF_NOARP))
>> >  		return -EINVAL;
>> 
>> Not directly related to the patch in question.  It just made me aware of
>> this restriction...
>> 
>> I realize that I'm a few years late here, but what's with the IFF_NOARP?
>> Is that just because we can't do DAD for the token based addresses?  How
>> is that different from manually configuring the whole address?
>
> IFF_NOARP is kind of the equivalent to no neighbor discovery. If you set
> a token and never get in a router advertisement you never create a
> tokenized ip address, thus the feature is useless.

You can get router advertisements with IFF_NOARP. You cannot lookup L2
addresses, but the L3 prefix info is still as useful as with any other
interface.

Doing

 tshark -nVi any -f ip6

while bringing up the POINTOPOINT NOARP interface shown at the end shows
the expected RS and RA:

Frame 1: 64 bytes on wire (512 bits), 64 bytes captured (512 bits) on interface 0
    Interface id: 0 (any)
    Encapsulation type: Linux cooked-mode capture (25)
    Arrival Time: Apr  8, 2016 17:18:36.094554456 CEST
    [Time shift for this packet: 0.000000000 seconds]
    Epoch Time: 1460128716.094554456 seconds
    [Time delta from previous captured frame: 0.000000000 seconds]
    [Time delta from previous displayed frame: 0.000000000 seconds]
    [Time since reference or first frame: 0.000000000 seconds]
    Frame Number: 1
    Frame Length: 64 bytes (512 bits)
    Capture Length: 64 bytes (512 bits)
    [Frame is marked: False]
    [Frame is ignored: False]
    [Protocols in frame: sll:ethertype:ipv6:icmpv6]
Linux cooked capture
    Packet type: Sent by us (4)
    Link-layer address type: 65534
    Link-layer address length: 0
    Protocol: IPv6 (0x86dd)
Internet Protocol Version 6, Src: fe80::8019:47ef:17a1:8c38, Dst: ff02::2
    0110 .... = Version: 6
    .... 0000 0000 .... .... .... .... .... = Traffic class: 0x00 (DSCP: CS0, ECN: Not-ECT)
        .... 0000 00.. .... .... .... .... .... = Differentiated Services Codepoint: Default (0)
        .... .... ..00 .... .... .... .... .... = Explicit Congestion Notification: Not ECN-Capable Transport (0)
    .... .... .... 0000 0000 0000 0000 0000 = Flowlabel: 0x00000000
    Payload length: 8
    Next header: ICMPv6 (58)
    Hop limit: 255
    Source: fe80::8019:47ef:17a1:8c38
    Destination: ff02::2
    [Source GeoIP: Unknown]
    [Destination GeoIP: Unknown]
Internet Control Message Protocol v6
    Type: Router Solicitation (133)
    Code: 0
    Checksum: 0x1155 [correct]
    Reserved: 00000000

Frame 2: 120 bytes on wire (960 bits), 120 bytes captured (960 bits) on interface 0
    Interface id: 0 (any)
    Encapsulation type: Linux cooked-mode capture (25)
    Arrival Time: Apr  8, 2016 17:18:36.101806992 CEST
    [Time shift for this packet: 0.000000000 seconds]
    Epoch Time: 1460128716.101806992 seconds
    [Time delta from previous captured frame: 0.007252536 seconds]
    [Time delta from previous displayed frame: 0.007252536 seconds]
    [Time since reference or first frame: 0.007252536 seconds]
    Frame Number: 2
    Frame Length: 120 bytes (960 bits)
    Capture Length: 120 bytes (960 bits)
    [Frame is marked: False]
    [Frame is ignored: False]
    [Protocols in frame: sll:ethertype:ipv6:icmpv6]
Linux cooked capture
    Packet type: Unicast to us (0)
    Link-layer address type: 65534
    Link-layer address length: 0
    Protocol: IPv6 (0x86dd)
Internet Protocol Version 6, Src: fe80::a5a6:793:6bfe:ea1c, Dst: fe80::8019:47ef:17a1:8c38
    0110 .... = Version: 6
    .... 0000 0000 .... .... .... .... .... = Traffic class: 0x00 (DSCP: CS0, ECN: Not-ECT)
        .... 0000 00.. .... .... .... .... .... = Differentiated Services Codepoint: Default (0)
        .... .... ..00 .... .... .... .... .... = Explicit Congestion Notification: Not ECN-Capable Transport (0)
    .... .... .... 0000 0000 0000 0000 0000 = Flowlabel: 0x00000000
    Payload length: 64
    Next header: ICMPv6 (58)
    Hop limit: 255
    Source: fe80::a5a6:793:6bfe:ea1c
    Destination: fe80::8019:47ef:17a1:8c38
    [Source GeoIP: Unknown]
    [Destination GeoIP: Unknown]
Internet Control Message Protocol v6
    Type: Router Advertisement (134)
    Code: 0
    Checksum: 0x96fa [correct]
    Cur hop limit: 255
    Flags: 0x40
        0... .... = Managed address configuration: Not set
        .1.. .... = Other configuration: Set
        ..0. .... = Home Agent: Not set
        ...0 0... = Prf (Default Router Preference): Medium (0)
        .... .0.. = Proxy: Not set
        .... ..0. = Reserved: 0
    Router lifetime (s): 65535
    Reachable time (ms): 0
    Retrans timer (ms): 0
    ICMPv6 Option (Source link-layer address : 02:50:f3:00:01:00)
        Type: Source link-layer address (1)
        Length: 1 (8 bytes)
        Link-layer address: 02:50:f3:00:01:00
    ICMPv6 Option (MTU : 1500)
        Type: MTU (5)
        Length: 1 (8 bytes)
        Reserved
        MTU: 1500
    ICMPv6 Option (Prefix information : 2a02:2121:81:e578::/64)
        Type: Prefix information (3)
        Length: 4 (32 bytes)
        Prefix Length: 64
        Flag: 0xc0
            1... .... = On-link flag(L): Set
            .1.. .... = Autonomous address-configuration flag(A): Set
            ..0. .... = Router address flag(R): Not set
            ...0 0000 = Reserved: 0
        Valid Lifetime: 4294967295 (Infinity)
        Preferred Lifetime: 4294967295 (Infinity)
        Reserved
        Prefix: 2a02:2121:81:e578::


nemi:/tmp# ifconfig wwan0
wwan0     Link encap:UNSPEC  HWaddr 00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00  
          inet addr:10.135.186.66  P-t-P:10.135.186.66  Mask:255.255.255.252
          inet6 addr: 2a02:2121:81:e578:bce:7da1:24a5:af48/64 Scope:Global
          inet6 addr: fe80::8019:47ef:17a1:8c38/64 Scope:Link
          UP POINTOPOINT RUNNING NOARP MULTICAST  MTU:1500  Metric:1
          RX packets:3 errors:0 dropped:0 overruns:0 frame:0
          TX packets:3 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:1000 
          RX bytes:716 (716.0 B)  TX bytes:704 (704.0 B)



(the other end is an LTE modem here)


Bjørn

Re: [PATCH net-next] ipv6, token: allow for clearing the current device token

[Hannes Frederic Sowa]

On 08.04.2016 17:25, Bjørn Mork wrote:
> Hannes Frederic Sowa <hannes@stressinduktion.org> writes:
>
>> On Fri, Apr 8, 2016, at 16:18, Bjørn Mork wrote:
>>> Daniel Borkmann <daniel@iogearbox.net> writes:
>>>
>>>>
>>>>   	if (!token)
>>>>   		return -EINVAL;
>>>> -	if (ipv6_addr_any(token))
>>>> -		return -EINVAL;
>>>>   	if (dev->flags & (IFF_LOOPBACK | IFF_NOARP))
>>>>   		return -EINVAL;
>>>
>>> Not directly related to the patch in question.  It just made me aware of
>>> this restriction...
>>>
>>> I realize that I'm a few years late here, but what's with the IFF_NOARP?
>>> Is that just because we can't do DAD for the token based addresses?  How
>>> is that different from manually configuring the whole address?
>>
>> IFF_NOARP is kind of the equivalent to no neighbor discovery. If you set
>> a token and never get in a router advertisement you never create a
>> tokenized ip address, thus the feature is useless.
>
> You can get router advertisements with IFF_NOARP. You cannot lookup L2
> addresses, but the L3 prefix info is still as useful as with any other
> interface.

Of course router advertisements can be send and received with IFF_NOARP 
and probably we act on them as usual, as you showed. Looking in the 
source we don't really specify what those flags mean/do for IPv6. So I 
think you can assume that it is in there because of history.

I would absolutely not mind if you remove the limitation for IFF_ARP.

Bye,
Hannes

Re: [PATCH net-next] ipv6, token: allow for clearing the current device token

[Daniel Borkmann]

On 04/08/2016 05:36 PM, Hannes Frederic Sowa wrote:
> On 08.04.2016 17:25, Bjørn Mork wrote:
>> Hannes Frederic Sowa <hannes@stressinduktion.org> writes:
>>> On Fri, Apr 8, 2016, at 16:18, Bjørn Mork wrote:
>>>> Daniel Borkmann <daniel@iogearbox.net> writes:
>>>>
>>>>>       if (!token)
>>>>>           return -EINVAL;
>>>>> -    if (ipv6_addr_any(token))
>>>>> -        return -EINVAL;
>>>>>       if (dev->flags & (IFF_LOOPBACK | IFF_NOARP))
>>>>>           return -EINVAL;
>>>>
>>>> Not directly related to the patch in question.  It just made me aware of
>>>> this restriction...
>>>>
>>>> I realize that I'm a few years late here, but what's with the IFF_NOARP?
>>>> Is that just because we can't do DAD for the token based addresses?  How
>>>> is that different from manually configuring the whole address?
>>>
>>> IFF_NOARP is kind of the equivalent to no neighbor discovery. If you set
>>> a token and never get in a router advertisement you never create a
>>> tokenized ip address, thus the feature is useless.
>>
>> You can get router advertisements with IFF_NOARP. You cannot lookup L2
>> addresses, but the L3 prefix info is still as useful as with any other
>> interface.
>
> Of course router advertisements can be send and received with IFF_NOARP and probably we act on them as usual, as you showed. Looking in the source we don't really specify what those flags mean/do for IPv6. So I think you can assume that it is in there because of history.
>
> I would absolutely not mind if you remove the limitation for IFF_ARP.

Agreed me neither, the code should be able to handle it as far as I see.

Thanks,
Daniel

Re: [PATCH net-next] ipv6, token: allow for clearing the current device token

[David Miller]

From: Daniel Borkmann <daniel@iogearbox.net>
Date: Fri,  8 Apr 2016 15:55:00 +0200

> The original tokenized iid support implemented via f53adae4eae5 ("net: ipv6:
> add tokenized interface identifier support") didn't allow for clearing a
> device token as it was intended that this addressing mode was the only one
> active for globally scoped IPv6 addresses. Later we relaxed that restriction
> via 617fe29d45bd ("net: ipv6: only invalidate previously tokenized addresses"),
> and we should also allow for clearing tokens as there's no good reason why
> it shouldn't be allowed.
> 
> Fixes: 617fe29d45bd ("net: ipv6: only invalidate previously tokenized addresses")
> Reported-by: Robin H. Johnson <robbat2@gentoo.org>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>

Applied to net-next, thanks.