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

[Hannes Frederic Sowa]

On 11.07.2016 16:48, David Miller wrote:
> From: Bjørn Mork 
> Date: Mon, 11 Jul 2016 16:43:50 +0200
> 
>> 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 adding a new private netdevice flag
>> which disables automatic IPv6 link local address generation, and
>> making the flag default for "tun" devices.
>>
>> Setting an explicit addrgenmode will disable the flag, so userspace
>> can choose to enable automatic LL generation by selecting a suitable
>> addrgenmode.
>>
>> 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 
>> Reported-by: Jonas Lippuner 
>> Suggested-by: Hannes Frederic Sowa 
>> Cc: 吉藤英明 
>> Signed-off-by: Bjørn Mork 
> 
> What really irks me is that we "fixing" something without knowing what
> actually is the problem.
> 
> Someone needs to figure out exactly what is making the Juniper thing
> unhappy.  It really shouldn't care if a link local address is assigned
> to the tun device, this is fundamental ipv6 stuff.

I agree, but there could be a lot of factors affecting this. For example
we know start to send out an icmp router solicitation into the vpn as
soon as the link goes up, as well as initial mld change record. It could
very well trigger security software and kill the VPN (something I can
very well imagine right now).

I actually don't suspect something funny going on in the Juniper's Java
client itself, this would just be too strange. Unfortunately I don't
have access to such systems to check what could be the problem.

Bjorn send a list of things that could be tried.

Thanks,
Hannes

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

[Bjørn Mork]

Hannes,

I just realized that I missed you on the CC list of the v2 patch. Sorry.
It was definitely not my intention. I really appreciate the feedback you
have kindly provided before.

I guess you might have seen it in netdev anyway, but in case not, here
is a direct patchwork link: http://patchwork.ozlabs.org/patch/646958/

I'll try to be less sloppy about CCs the next time, if we ever get to a
v3 of this. At the moment that requires figuring out what the actual
problem with the client is. Something I don't have the necessary tools
to do AFAICS.  So I will let the whole thing just sleep until there is
some more data about the Juniper client failure.


Bjørn

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

[Bjørn Mork]

David Miller  writes:

> What really irks me is that we "fixing" something without knowing what
> actually is the problem.

Agreed

> Someone needs to figure out exactly what is making the Juniper thing
> unhappy.  It really shouldn't care if a link local address is assigned
> to the tun device, this is fundamental ipv6 stuff.

Yes. Looks like this is up to Jonas and/or Valdis.  I tried looking for
a demo site which could be used to test the client, but could not find
any.  The product itself seems to be replaced, and it's no longer Juniper.
And the recommended Linux solution seems to be OpenConnect:
http://www.infradead.org/openconnect/juniper.html

Anyway, it would be good to sort out the problems with the java(?) based
client.  A few proposals (not an exhaustive list - please use your
creativity):

a) Try to figure out what the traffic on the interface looks like (there
   was a single TX packet and no RX, I believe?).  Snoop on it and see
   if that is an IPv6 RS from the kernel or something the client sends.

b) Try to isolate the problem by tweaking what you can on the tun-
   interface.

 ip addr del  dev tun0
 echo 1 > /proc/sys/net/ipv6/conf/tun0/disable_ipv6

  etc.  Is there anything that will make the traffic flow, or is it just
  dead?


c) Try to figure out what the client is doing.  strace it.  run lsof on
   it.  Anything unexpected?  Does it for example happen to read an
   packet from the tun file descriptor and choke?


etc,



Bjørn

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

[David Miller]

From: Bjørn Mork 
Date: Mon, 11 Jul 2016 16:43:50 +0200

> 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 adding a new private netdevice flag
> which disables automatic IPv6 link local address generation, and
> making the flag default for "tun" devices.
> 
> Setting an explicit addrgenmode will disable the flag, so userspace
> can choose to enable automatic LL generation by selecting a suitable
> addrgenmode.
> 
> 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 
> Reported-by: Jonas Lippuner 
> Suggested-by: Hannes Frederic Sowa 
> Cc: 吉藤英明 
> Signed-off-by: Bjørn Mork 

What really irks me is that we "fixing" something without knowing what
actually is the problem.

Someone needs to figure out exactly what is making the Juniper thing
unhappy.  It really shouldn't care if a link local address is assigned
to the tun device, this is fundamental ipv6 stuff.

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

[Bjørn Mork]

valdis.kletni...@vt.edu writes:

> Tested against next-20160708, and the Juniper code works fine. Feel free
> to stick a Tested-By: on the V2 patch...

Thanks to both of you for verifying that it solves the Juniper problem.

A tip: Patchworks is nice enough to automatically pick up tags from
review comments, as long as the tags start at column 0.  I haven't tried
multiple tags, but let's see if this works:

Tested-by: Jonas Lippuner 
Tested-by: Valdis Kletnieks 


Bjørn

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

[Valdis . Kletnieks]

On Mon, 11 Jul 2016 16:43:50 +0200, Bjørn Mork said:

> 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 
> Reported-by: Jonas Lippuner 
> Suggested-by: Hannes Frederic Sowa 
> Cc: 吉藤英明 
> Signed-off-by: Bjørn Mork 
> ---
> v2 changes:
>  - added a netdevice private flag to suppress automatic IPv6 LL
>  - suppressing only for "tun" devices

Tested against next-20160708, and the Juniper code works fine. Feel free
to stick a Tested-By: on the V2 patch...


pgpFM6HCMatPo.pgp
Description: PGP signature

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

[Valdis . Kletnieks]

On Mon, 11 Jul 2016 16:43:50 +0200, Bjørn Mork said:

> And finally, Valdis and Jonas: could you please test this version too? It
> works for me in my simulated setup, but I don't have the Juniper client
> so I cannot verify that it actually solves the problem.

The v1 patch worked.  I'll be able to test the v2 patch in a few hours


pgpV4AP3Vk0Uu.pgp
Description: PGP signature

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

[Jonas Lippuner]

Thanks Bjørn!

I have applied the patch to v4.6.3 of linux-stable and Juniper VPN works
with the patch. Please let me know if I should test the patch applied to
any other code revision.


Best,
Jonas


 Original Message  
Subject: [PATCH v2 net] ipv6: addrconf: fix Juniper SSL VPN client
regression
From: Bjørn Mork <bj...@mork.no>
To: netdev@vger.kernel.org
CC: Valdis Kletnieks <valdis.kletni...@vt.edu>, Jonas Lippuner
<jo...@lippuner.ca>, Bjørn Mork <bj...@mork.no>, 吉藤英明
<hideaki.yoshif...@miraclelinux.com>
Date: 7/11/2016, 7:43:50 AM

> 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 adding a new private netdevice flag
> which disables automatic IPv6 link local address generation, and
> making the flag default for "tun" devices.
> 
> Setting an explicit addrgenmode will disable the flag, so userspace
> can choose to enable automatic LL generation by selecting a suitable
> addrgenmode.
> 
> 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.kletni...@vt.edu>
> Reported-by: Jonas Lippuner <jo...@lippuner.ca>
> Suggested-by: Hannes Frederic Sowa <han...@stressinduktion.org>
> Cc: 吉藤英明 <hideaki.yoshif...@miraclelinux.com>
> Signed-off-by: Bjørn Mork <bj...@mork.no>
> ---
> v2 changes:
>  - added a netdevice private flag to suppress automatic IPv6 LL
>  - suppressing only for "tun" devices
> 
> 
> So, something like this?  It has the bonus that it can be used for *any*
> type of device which does not want the automatic link local addresses.
> Only enabled for "tun" for now, of course.
> 
> Is it OK to unconditionally disable the suppression if the user sets an
> addrgenmode?  I find that to match *my* expectations, but I don't know
> much about the ordinary user :)
> 
> And finally, Valdis and Jonas: could you please test this version too? It
> works for me in my simulated setup, but I don't have the Juniper client
> so I cannot verify that it actually solves the problem.
> 
> 
> Bjørn
> 
> 
>  drivers/net/tun.c | 4 
>  include/linux/netdevice.h | 4 
>  net/ipv6/addrconf.c   | 7 +++
>  3 files changed, 15 insertions(+)
> 
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index e16487cc6a9a..6e7558f97013 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -1073,6 +1073,10 @@ static void tun_net_init(struct net_device *dev)
>   /* Zero header length */
>   dev->type = ARPHRD_NONE;
>   dev->flags = IFF_POINTOPOINT | IFF_NOARP | IFF_MULTICAST;
> +
> + /* IPv6 LL address is known to break some applications */
> + dev->priv_flags |= IFF_SUPPRESS_AUTO_IPV6_LL;
> +
>   break;
>  
>   case IFF_TAP:
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index f45929ce8157..d04ea7fcdaba 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -1333,6 +1333,8 @@ struct net_device_ops {
>   * @IFF_PHONY_HEADROOM: the headroom value is controlled by an external
>   *   entity (i.e. the master device for bridged veth)
>   * @IFF_MACSEC: device is a MACsec device
> + * @IFF_SUPPRESS_AUTO_IPV6_LL: device will not get an automatic IPv6
> + *   link local address
>   */
>  enum netdev_priv_flags {
>   IFF_802_1Q_VLAN = 1<<0,
> @@ -1363,6 +1365,7 @@ enum netdev_priv_flags {
>   IFF_RXFH_CONFIGURED = 1<<25,
>   IFF_PHONY_HEADROOM  = 1<<26,
>   IFF_MACSEC  = 1<<27,
> + IFF_SUPPRESS_AUTO_IPV6_LL   = 1<<28,
>  };
>  
>  #define IFF_802_1Q_VLAN  IFF_802_1Q_VLAN
> @@ -1392,6 +1395,7 @@ enum netdev_priv_flags {
>  #define IFF_TEAM IFF_TEAM
>  #define IFF_RXFH_CONFIGURED  IFF_RXFH_CONFIGURED
>  #define IFF_MACSEC   IFF_MACSEC
> +#define IFF_SUPPRESS_AUTO_IPV6_LLIFF_SUPPRESS_AUTO_IPV6_LL
>  
>  /**
>   *   struct net_device - The DEVICE structure.
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 47f837a58e0a..331ea5ebff5f 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net

[PATCH v2 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 adding a new private netdevice flag
which disables automatic IPv6 link local address generation, and
making the flag default for "tun" devices.

Setting an explicit addrgenmode will disable the flag, so userspace
can choose to enable automatic LL generation by selecting a suitable
addrgenmode.

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 
Reported-by: Jonas Lippuner 
Suggested-by: Hannes Frederic Sowa 
Cc: 吉藤英明 
Signed-off-by: Bjørn Mork 
---
v2 changes:
 - added a netdevice private flag to suppress automatic IPv6 LL
 - suppressing only for "tun" devices


So, something like this?  It has the bonus that it can be used for *any*
type of device which does not want the automatic link local addresses.
Only enabled for "tun" for now, of course.

Is it OK to unconditionally disable the suppression if the user sets an
addrgenmode?  I find that to match *my* expectations, but I don't know
much about the ordinary user :)

And finally, Valdis and Jonas: could you please test this version too? It
works for me in my simulated setup, but I don't have the Juniper client
so I cannot verify that it actually solves the problem.


Bjørn


 drivers/net/tun.c | 4 
 include/linux/netdevice.h | 4 
 net/ipv6/addrconf.c   | 7 +++
 3 files changed, 15 insertions(+)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index e16487cc6a9a..6e7558f97013 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1073,6 +1073,10 @@ static void tun_net_init(struct net_device *dev)
/* Zero header length */
dev->type = ARPHRD_NONE;
dev->flags = IFF_POINTOPOINT | IFF_NOARP | IFF_MULTICAST;
+
+   /* IPv6 LL address is known to break some applications */
+   dev->priv_flags |= IFF_SUPPRESS_AUTO_IPV6_LL;
+
break;
 
case IFF_TAP:
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index f45929ce8157..d04ea7fcdaba 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1333,6 +1333,8 @@ struct net_device_ops {
  * @IFF_PHONY_HEADROOM: the headroom value is controlled by an external
  * entity (i.e. the master device for bridged veth)
  * @IFF_MACSEC: device is a MACsec device
+ * @IFF_SUPPRESS_AUTO_IPV6_LL: device will not get an automatic IPv6
+ * link local address
  */
 enum netdev_priv_flags {
IFF_802_1Q_VLAN = 1<<0,
@@ -1363,6 +1365,7 @@ enum netdev_priv_flags {
IFF_RXFH_CONFIGURED = 1<<25,
IFF_PHONY_HEADROOM  = 1<<26,
IFF_MACSEC  = 1<<27,
+   IFF_SUPPRESS_AUTO_IPV6_LL   = 1<<28,
 };
 
 #define IFF_802_1Q_VLANIFF_802_1Q_VLAN
@@ -1392,6 +1395,7 @@ enum netdev_priv_flags {
 #define IFF_TEAM   IFF_TEAM
 #define IFF_RXFH_CONFIGUREDIFF_RXFH_CONFIGURED
 #define IFF_MACSEC IFF_MACSEC
+#define IFF_SUPPRESS_AUTO_IPV6_LL  IFF_SUPPRESS_AUTO_IPV6_LL
 
 /**
  * struct net_device - The DEVICE structure.
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 47f837a58e0a..331ea5ebff5f 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -3113,6 +3113,10 @@ static void addrconf_dev_config(struct net_device *dev)
return;
}
 
+   /* this device does not want automatic IPv6 LLs */
+   if (dev->priv_flags & IFF_SUPPRESS_AUTO_IPV6_LL)
+   return;
+
idev = addrconf_add_dev(dev);
if (IS_ERR(idev))
return;
@@ -5104,6 +5108,9 @@ static int inet6_set_link_af(struct net_device *dev, 
const struct nlattr *nla)
 
idev->addr_gen_mode = mode;
err = 0;
+
+   /* turn off suppression since user has requested addrgen */
+   dev->priv_flags &= ~IFF_SUPPRESS_AUTO_IPV6_LL;
}
 
return err;
-- 
2.8.1