[PATCH net] ipv6: addrconf: fix Juniper SSL VPN client regression

[PATCH net] ipv6: addrconf: fix Juniper SSL VPN client regression

[Bjørn Mork]

The Juniper SSL VPN client use a "tun" interface and seems to
be picky about visible changes.to it. Commit cc9da6cc4f56
("ipv6: addrconf: use stable address generator for ARPHRD_NONE")
made such interfaces get an auto-generated IPv6 link local address
by default, similar to most other interface types. This made the
Juniper SSL VPN client fail for unknown reasons.

Fixing this regression by effectively reverting the behaviour to
what we had before, while keeping the new "addrgenmode random"
feature.

This will cause a regression for any userspace application which
has started to depend on the new behaviour.  But it is still
considered the lesser evil, considering the short period this
behaviour has been default.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=121131
Fixes: cc9da6cc4f56 ("ipv6: addrconf: use stable address generator for ARPHRD_NONE")
Reported-by: Valdis Kletnieks <Valdis.Kletnieks@vt.edu>
Reported-and-tested-by: Jonas Lippuner <jonas@lippuner.ca>
Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
Cc: 吉藤英明 <hideaki.yoshifuji@miraclelinux.com>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
Valdis,

I know you reported this regression back in April, but I never
saw any answers to Hannes' and mine followup quesions so I forgot
all about it.  Sorry about that.  Could you test this patch and
see if it works for you too?


Bjørn

 net/ipv6/addrconf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 47f837a58e0a..34e80c6aa810 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -3120,7 +3120,7 @@ static void addrconf_dev_config(struct net_device *dev)
 	/* this device type has no EUI support */
 	if (dev->type == ARPHRD_NONE &&
 	    idev->addr_gen_mode == IN6_ADDR_GEN_MODE_EUI64)
-		idev->addr_gen_mode = IN6_ADDR_GEN_MODE_RANDOM;
+		idev->addr_gen_mode = IN6_ADDR_GEN_MODE_NONE;
 
 	addrconf_addr_gen(idev, false);
 }
-- 
2.8.1

Re: [PATCH net] ipv6: addrconf: fix Juniper SSL VPN client regression

[Hannes Frederic Sowa]

Hi Bjorn,

On Sat, Jul 9, 2016, at 23:13, Bjørn Mork wrote:
> The Juniper SSL VPN client use a "tun" interface and seems to
> be picky about visible changes.to it. Commit cc9da6cc4f56
> ("ipv6: addrconf: use stable address generator for ARPHRD_NONE")
> made such interfaces get an auto-generated IPv6 link local address
> by default, similar to most other interface types. This made the
> Juniper SSL VPN client fail for unknown reasons.
> 
> Fixing this regression by effectively reverting the behaviour to
> what we had before, while keeping the new "addrgenmode random"
> feature.

I wonder if we can simply add a flag, something like
IFF_SUPPRESS_AUTO_IPV6_LL, to net_device->priv_flags and use that. So we
can keep behavior for qmi, vxlan-gpe and gre. tun is the only device
that is really user space facing, so maybe we just limit it to this?

Thanks,
Hannes

Re: [PATCH net] ipv6: addrconf: fix Juniper SSL VPN client regression

[Bjørn Mork]

Hannes Frederic Sowa <hannes@stressinduktion.org> writes:
> On Sat, Jul 9, 2016, at 23:13, Bjørn Mork wrote:
>> The Juniper SSL VPN client use a "tun" interface and seems to
>> be picky about visible changes.to it. Commit cc9da6cc4f56
>> ("ipv6: addrconf: use stable address generator for ARPHRD_NONE")
>> made such interfaces get an auto-generated IPv6 link local address
>> by default, similar to most other interface types. This made the
>> Juniper SSL VPN client fail for unknown reasons.
>> 
>> Fixing this regression by effectively reverting the behaviour to
>> what we had before, while keeping the new "addrgenmode random"
>> feature.
>
> I wonder if we can simply add a flag, something like
> IFF_SUPPRESS_AUTO_IPV6_LL, to net_device->priv_flags and use that. So we
> can keep behavior for qmi, vxlan-gpe and gre. tun is the only device
> that is really user space facing, so maybe we just limit it to this?

Sounds good to me, but I don't know if the use case really qualifies as

 "* You should have a pretty good reason to be extending these flags."


The automatic address is certainly nice to have, but "good reason"?  I
don't know...  We can always just configure those devices for automatic
LL addresses using "ip link set foo addrgen random" or similar.


Bjørn

Re: [PATCH net] ipv6: addrconf: fix Juniper SSL VPN client regression

[Hannes Frederic Sowa]

On 09.07.2016 19:23, Bjørn Mork wrote:
> Hannes Frederic Sowa <hannes@stressinduktion.org> writes:
>> On Sat, Jul 9, 2016, at 23:13, Bjørn Mork wrote:
>>> The Juniper SSL VPN client use a "tun" interface and seems to
>>> be picky about visible changes.to it. Commit cc9da6cc4f56
>>> ("ipv6: addrconf: use stable address generator for ARPHRD_NONE")
>>> made such interfaces get an auto-generated IPv6 link local address
>>> by default, similar to most other interface types. This made the
>>> Juniper SSL VPN client fail for unknown reasons.
>>>
>>> Fixing this regression by effectively reverting the behaviour to
>>> what we had before, while keeping the new "addrgenmode random"
>>> feature.
>>
>> I wonder if we can simply add a flag, something like
>> IFF_SUPPRESS_AUTO_IPV6_LL, to net_device->priv_flags and use that. So we
>> can keep behavior for qmi, vxlan-gpe and gre. tun is the only device
>> that is really user space facing, so maybe we just limit it to this?
> 
> Sounds good to me, but I don't know if the use case really qualifies as
> 
>  "* You should have a pretty good reason to be extending these flags."
> 
> 
> The automatic address is certainly nice to have, but "good reason"?  I
> don't know...  We can always just configure those devices for automatic
> LL addresses using "ip link set foo addrgen random" or similar.

I do think it is important enough to include it into priv_flags,
especially if you compare it to other flags in there. We also can easily
add new priv_flags member or enlarge to long if we run out in the long term.

I would slightly prefer if tunnels keep the behavior we introduced one
release(?) ago.

Thanks,
Hannes

Re: [PATCH net] ipv6: addrconf: fix Juniper SSL VPN client regression

[Bjørn Mork]

Hannes Frederic Sowa <hannes@stressinduktion.org> writes:
> On 09.07.2016 19:23, Bjørn Mork wrote:
>> Hannes Frederic Sowa <hannes@stressinduktion.org> writes:
>>> On Sat, Jul 9, 2016, at 23:13, Bjørn Mork wrote:
>>>> The Juniper SSL VPN client use a "tun" interface and seems to
>>>> be picky about visible changes.to it. Commit cc9da6cc4f56
>>>> ("ipv6: addrconf: use stable address generator for ARPHRD_NONE")
>>>> made such interfaces get an auto-generated IPv6 link local address
>>>> by default, similar to most other interface types. This made the
>>>> Juniper SSL VPN client fail for unknown reasons.
>>>>
>>>> Fixing this regression by effectively reverting the behaviour to
>>>> what we had before, while keeping the new "addrgenmode random"
>>>> feature.
>>>
>>> I wonder if we can simply add a flag, something like
>>> IFF_SUPPRESS_AUTO_IPV6_LL, to net_device->priv_flags and use that. So we
>>> can keep behavior for qmi, vxlan-gpe and gre. tun is the only device
>>> that is really user space facing, so maybe we just limit it to this?
>> 
>> Sounds good to me, but I don't know if the use case really qualifies as
>> 
>>  "* You should have a pretty good reason to be extending these flags."
>> 
>> 
>> The automatic address is certainly nice to have, but "good reason"?  I
>> don't know...  We can always just configure those devices for automatic
>> LL addresses using "ip link set foo addrgen random" or similar.
>
> I do think it is important enough to include it into priv_flags,
> especially if you compare it to other flags in there. We also can easily
> add new priv_flags member or enlarge to long if we run out in the long term.

OK, I'll cook a new version with that.


> I would slightly prefer if tunnels keep the behavior we introduced one
> release(?) ago.

Depends on how you count.  it was introduced in v4.5.


Bjørn