New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 768053 link

Starred by 4 users

Issue metadata

Status: Assigned
Owner:
Last visit > 30 days ago
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

Notify ARC VPN on physical connection changes

Project Member Reported by cernekee@chromium.org, Sep 22 2017

Issue description

If the system is connected to an ARC VPN and the underlying physical connection (ethernet, wifi, cellular) changes state, we do not currently notify Android.  This is because the ARC VPN is the default service, so ArcNetHostImpl::DefaultNetworkChanged will not get invoked if the *underlying* service changes.

It would be good if we notified Android of these connection changes, because then the Android VPN app can make more informed decisions about when to disconnect or reconnect the VPN, rather than allowing a "dead" connection to retry and time out.

(It might make sense to roll this into the multinetwork project rather than trying to fix the code we have today.)
 
Another side effect of this is that if the LAN's DNS servers change (because the Chromebook switched from wired<->wireless, for instance), Android might not find out about it.  This could affect the VPN app's ability to reconnect, and it could also affect apps that are bypassing the VPN because the per-app VPN feature (addAllowedApplication / addDisallowedApplication) is in use.
The observer method you want to listen to instead of DefaultNetworkChanged() is NetwworkStateHandlerObserver::NetworkConnectionStateChanged(network), then compare |network| to FirstNetworkByTyoe(NonVirtual()).


Labels: ChromeOsVpn
Components: OS>Systems>Network Internals>Network>VPN
> NetwworkStateHandlerObserver::NetworkConnectionStateChanged(network), then compare |network| to FirstNetworkByTyoe(NonVirtual()).

Hi Steven, this does not seem to be working for me.  I am not getting a callback from either NetworkConnectionStateChanged or DefaultNetworkChanged when the physical connection change is complete.  I am getting a callback when the existing default physical connection goes offline, but not when the new one goes online.

Is there a different callback that will help here?

My WIP CL: https://chromium-review.googlesource.com/#/c/chromium/src/+/757679
That sounds like a bug, can you maybe investigate, look at chrome://device-log?

The code that should be triggering the observer is here:
https://cs.chromium.org/chromium/src/chromeos/network/network_state_handler.cc?type=cs&q=NetworkConnectionStateChanged&sq=package:chromium&l=1604
Project Member

Comment 7 by bugdroid1@chromium.org, Nov 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/583cd55a4300e4ea50356f7921b3c62acf2056e7

commit 583cd55a4300e4ea50356f7921b3c62acf2056e7
Author: Kevin Cernekee <cernekee@chromium.org>
Date: Sat Nov 18 02:04:23 2017

Fix ConnectedNetworkByType() results during state transitions

If wifi and ethernet are both connected, and then the ethernet adapter
is unplugged, network_list_ can end up looking like:

    network_list_[0] -> ethernet, idle
    network_list_[1] -> wifi, online

shill will generally update the service's State property before it
updates the manager's Services / ServiceCompleteList properties,
leaving the now-idle service in the list.

Since ConnectedNetworkByType() assumes that connected networks are
listed first, it won't return the newly-active wifi network.  Fix
this by sorting the network list, and changing the sort function so
that it doesn't prioritize idle ethernet services.

BUG=768053
TEST=manually verify order with debug prints
TEST=unit tests

Change-Id: I3a110da7d99bd5e6463b86e17bc557124e875b85
Reviewed-on: https://chromium-review.googlesource.com/777970
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Kevin Cernekee <cernekee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517679}
[modify] https://crrev.com/583cd55a4300e4ea50356f7921b3c62acf2056e7/chromeos/network/network_state_handler.cc
[modify] https://crrev.com/583cd55a4300e4ea50356f7921b3c62acf2056e7/chromeos/network/network_state_handler.h

Project Member

Comment 8 by bugdroid1@chromium.org, Nov 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9c18d1f8cfa4855034c6655e406938b804e8548d

commit 9c18d1f8cfa4855034c6655e406938b804e8548d
Author: Kevin Cernekee <cernekee@chromium.org>
Date: Sat Nov 25 00:54:40 2017

Notify ARC of physical connection changes when ARC VPN is up

If ARC VPN is up, Chrome should send notifications whenever the
underlying physical network connection (wifi, ethernet, cellular)
changes so that the VPN app can trigger a reconnection.

BUG=768053
TEST=manually connect/disconnect from VPN, ethernet, wifi and observe logs

Change-Id: I64ad4955c96d2eb2a66a9cc2bf12bb82b69c2418
Reviewed-on: https://chromium-review.googlesource.com/757679
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Elijah Taylor <elijahtaylor@chromium.org>
Commit-Queue: Kevin Cernekee <cernekee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519172}
[modify] https://crrev.com/9c18d1f8cfa4855034c6655e406938b804e8548d/components/arc/net/arc_net_host_impl.cc
[modify] https://crrev.com/9c18d1f8cfa4855034c6655e406938b804e8548d/components/arc/net/arc_net_host_impl.h

Project Member

Comment 9 by bugdroid1@chromium.org, Dec 1 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/871e1bd910a31bb7cbdb831c08d4344f5da1c6b4

commit 871e1bd910a31bb7cbdb831c08d4344f5da1c6b4
Author: Kevin Cernekee <cernekee@chromium.org>
Date: Fri Dec 01 23:32:48 2017

Disconnect ARC VPN if ARC++ is shut down

Currently, if ARC++ is uninstalled while an Android VPN is running,
shill will not receive a disconnection event and this will break
networking on Chrome OS.  Fix this by forcing ARC VPN to disconnect
whenever ArcNetHostImpl shuts down.

BUG=768053
TEST=uninstall ARC++ through Settings with a VPN connected

Change-Id: Iccbdc20bb26bce154f8041a517b1220519a2b738
Reviewed-on: https://chromium-review.googlesource.com/778340
Commit-Queue: Kevin Cernekee <cernekee@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Elijah Taylor <elijahtaylor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521121}
[modify] https://crrev.com/871e1bd910a31bb7cbdb831c08d4344f5da1c6b4/components/arc/net/arc_net_host_impl.cc

Cc: nohle@chromium.org ofodile@chromium.org
Cc: -nohle@chromium.org nohe@chromium.org
Labels: Hotlist-CEPartnerEng
Status: Assigned (was: Unconfirmed)
This issue has an owner, a component and a priority, but is still listed as untriaged or unconfirmed. By definition, this bug is triaged. Changing status to "assigned". Please reach out to me if you disagree with how I've done this.

Sign in to add a comment