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

[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 <Valdis.Kletnieks@vt.edu>
Reported-by: Jonas Lippuner <jonas@lippuner.ca>
Suggested-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
Cc: 吉藤英明 <hideaki.yoshifuji@miraclelinux.com>
Signed-off-by: Bjørn Mork <bjorn@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_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

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

[Jonas Lippuner]

[-- Attachment #1.1: Type: text/plain, Size: 5250 bytes --]

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 <bjorn@mork.no>
To: netdev@vger.kernel.org
CC: Valdis Kletnieks <Valdis.Kletnieks@vt.edu>, Jonas Lippuner
<jonas@lippuner.ca>, Bjørn Mork <bjorn@mork.no>, 吉藤英明
<hideaki.yoshifuji@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.Kletnieks@vt.edu>
> Reported-by: Jonas Lippuner <jonas@lippuner.ca>
> Suggested-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Cc: 吉藤英明 <hideaki.yoshifuji@miraclelinux.com>
> Signed-off-by: Bjørn Mork <bjorn@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_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;
> 

-- 
My email is signed and I encrypt email on request.
To verify my signature or send me encrypted email,
get my public key: http://lippuner.ca/key


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 345 bytes --]

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....

[-- Attachment #2: Type: application/pgp-signature, Size: 848 bytes --]

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

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 733 bytes --]

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 <Valdis.Kletnieks@vt.edu>
> Reported-by: Jonas Lippuner <jonas@lippuner.ca>
> Suggested-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Cc: 吉藤英明 <hideaki.yoshifuji@miraclelinux.com>
> Signed-off-by: Bjørn Mork <bjorn@mork.no>
> ---
> 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...

[-- Attachment #2: Type: application/pgp-signature, Size: 848 bytes --]

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

[Bjørn Mork]

Valdis.Kletnieks@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 <jonas@lippuner.ca>
Tested-by: Valdis Kletnieks <Valdis.Kletnieks@vt.edu>


Bjørn

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

[David Miller]

From: Bjørn Mork <bjorn@mork.no>
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 <Valdis.Kletnieks@vt.edu>
> Reported-by: Jonas Lippuner <jonas@lippuner.ca>
> Suggested-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Cc: 吉藤英明 <hideaki.yoshifuji@miraclelinux.com>
> Signed-off-by: Bjørn Mork <bjorn@mork.no>

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]

David Miller <davem@davemloft.net> 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 <ipv6ll> 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

[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

[Hannes Frederic Sowa]

On 11.07.2016 16:48, David Miller wrote:
> From: Bjørn Mork <bjorn@mork.no>
> 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 <Valdis.Kletnieks@vt.edu>
>> Reported-by: Jonas Lippuner <jonas@lippuner.ca>
>> Suggested-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
>> Cc: 吉藤英明 <hideaki.yoshifuji@miraclelinux.com>
>> Signed-off-by: Bjørn Mork <bjorn@mork.no>
> 
> 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