VPN reconnections can fail when using a captive portal |
||||
Issue descriptionThe current logic in shill's Manager will execute all default service callbacks under these conditions: - The default physical service changes - The default physical service gains or loses its connection() In order to inform a VPN client that captive portal detection has passed, the VPN code in shill needs to find out when the default service transitions from Portal->Online. This event does not trigger either of the above conditions. So currently, VPN apps can receive a linkUp event when the default service is in the Connected/Portal state (i.e. somebody just resumed their laptop at a coffee shop). Ideally the linkUp event would happen when the default service transitions from Connected->Online or Portal->Online, instead. This would avoid the need for the VPN client to keep retrying at a time when internet connectivity is unavailable. Also, we should add new test cases to exercise the various link/power state transitions.
,
Mar 30 2016
One caveat: While we are in captive portal state and the VPN is not actually connected, the UI will probably indicate that it *is* connected, leading to a false sense of security.
,
Mar 30 2016
The UI shows it as "Reconnecting" and the VPN badge will pulsate, similar to what happens when the VPN is initially connecting. When the app calls setParameters again, the UI will show "Connected" again.
,
Mar 31 2016
Ah, cool. That sounds perfect.
,
Mar 31 2016
#CBC-RS/TC-watchlist
,
May 15 2016
Landed fixes: https://android-review.googlesource.com/#/c/221533 https://android-review.googlesource.com/#/c/221534 shill uprev is under review: https://chromium-review.googlesource.com/#/c/344772
,
May 17 2016
,
May 18 2016
The following revision refers to this bug: https://chromium.googlesource.com/aosp/platform/system/connectivity/shill/+/37312ff7e894a39d330a7879f8819ad721028686 commit 37312ff7e894a39d330a7879f8819ad721028686 Author: Kevin Cernekee <cernekee@google.com> Date: Mon May 02 05:37:06 2016 Initate VPN reconnections when the default physical service is Online The current logic tries to reconnect the VPN when the default physical service changes, and is in any of the connected states. This means that it will try to reconnect even if a portal was detected and shill is waiting for the user to authenticate to the portal. This is not desirable because the device does not have internet access in Portal state. Change it so that it waits until the default service is Online before attempting a reconnection. Bug: None BUG= chromium:598781 TEST=`FEATURES=test emerge-link shill` (cherry picked from commit 8e3ac1b9da21aac8c0e97049e0a0c8326f32e3ba) Change-Id: I42d6a90e16e5b3a3834fba6e0e249b0d45226e03 Reviewed-on: https://chromium-review.googlesource.com/345706 Commit-Ready: Alex Deymo <deymo@chromium.org> Tested-by: Alex Deymo <deymo@chromium.org> Reviewed-by: Kevin Cernekee <cernekee@chromium.org> [modify] https://crrev.com/37312ff7e894a39d330a7879f8819ad721028686/vpn/third_party_vpn_driver.h [modify] https://crrev.com/37312ff7e894a39d330a7879f8819ad721028686/vpn/vpn_service.h [modify] https://crrev.com/37312ff7e894a39d330a7879f8819ad721028686/vpn/vpn_service.cc [modify] https://crrev.com/37312ff7e894a39d330a7879f8819ad721028686/vpn/third_party_vpn_driver.cc [modify] https://crrev.com/37312ff7e894a39d330a7879f8819ad721028686/vpn/third_party_vpn_driver_unittest.cc [modify] https://crrev.com/37312ff7e894a39d330a7879f8819ad721028686/vpn/vpn_driver.h [modify] https://crrev.com/37312ff7e894a39d330a7879f8819ad721028686/vpn/vpn_driver.cc
,
May 18 2016
The following revision refers to this bug: https://chromium.googlesource.com/aosp/platform/system/connectivity/shill/+/9da17b533987cb8807afabad7a03d7952b1e9923 commit 9da17b533987cb8807afabad7a03d7952b1e9923 Author: Kevin Cernekee <cernekee@google.com> Date: Mon May 02 03:33:47 2016 Add Service::OnDefaultServiceStateChanged() callback This allows VPNs to figure out when the default physical service has transitioned back to the Online state, so they can trigger a reconnection. Bug: None BUG= chromium:598781 TEST=`FEATURES=test emerge-link shill` (cherry picked from commit e5fb49474296421454309d1995890613a0da9ceb) Change-Id: I7072415a06238c7db96528cfcc13d1e38a9613e2 Reviewed-on: https://chromium-review.googlesource.com/345705 Commit-Ready: Alex Deymo <deymo@chromium.org> Tested-by: Alex Deymo <deymo@chromium.org> Reviewed-by: Kevin Cernekee <cernekee@chromium.org> [modify] https://crrev.com/9da17b533987cb8807afabad7a03d7952b1e9923/manager_unittest.cc [modify] https://crrev.com/9da17b533987cb8807afabad7a03d7952b1e9923/service.cc [modify] https://crrev.com/9da17b533987cb8807afabad7a03d7952b1e9923/service.h [modify] https://crrev.com/9da17b533987cb8807afabad7a03d7952b1e9923/mock_service.h [modify] https://crrev.com/9da17b533987cb8807afabad7a03d7952b1e9923/manager.h [modify] https://crrev.com/9da17b533987cb8807afabad7a03d7952b1e9923/manager.cc
,
May 18 2016
,
May 19 2016
cernekee, is this the fix that will also remove the redundant linkDown messages?
,
May 19 2016
Yes
,
May 23 2016
Bulk verified |
||||
►
Sign in to add a comment |
||||
Comment 1 by cernekee@chromium.org
, Mar 29 2016