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

Issue 850550 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Fix AutoConnectHandler for AllowOnlyPolicyNetworksTo[Auto]Connect

Project Member Reported by hendrich@chromium.org, Jun 7 2018

Issue description

Chrome Version: 69.0.3449.0
OS: ChromeOS (10756.0.0)

The AllowOnlyPolicyNetworksToConnect (and AllowOnlyPolicyNetworksToAutoconnect) policy is not enforced in Shill, only in chrome. This is a problem, as we can easily auto-connect to a previously connected unmanaged network. Additionally, due to another bug, the unmanaged network is not even disconnected once the policy is applied.

Scenario:
1) Connect to unmanaged network "U".
2) Set up device policy with AllowOnlyPolicyNetworksToConnect=true and provide a network configuration for the managed network "M".
3) Reload policies on device.
4) The device does not even disconnect from the unmanaged network! (unexpected behavior due to applied_autoconnect_policy_ implemented incorrectly)
5) Manually connect to network M.
6) Manual connections to U are not possible now (expected behavior)
7) Manually disconnect from network M.
8) AutoConnect will re-connect to network U (unexpected behavior)

So apart from fixing the existing bug in 4), we also need to prevent Shill from connecting to the disallowed networks when AllowOnlyPolicyNetworksToConnect (or AllowOnlyPolicyNetworksToAutoconnect) is enabled.

We can do this by either disabling auto-connect for all unmanaged network configurations, which prevents Shill from re-connecting to those networks again. For the AllowOnlyPolicyNetworksToConnect policy, we could even delete the network configuration, although disabling auto-connect is enough.

I have already started workin on a fix for both problems.

 
Rather than disabling auto-connect for unmanaged networks, shouldn't we just forget them?

For AllowOnlyPolicyNetworksToConnect we could aswell remove/forget them entirely, if you prefer.
For AllowOnlyPolicyNetworksToAutoconnect we would have to remember them and only disable auto-connect to still enable manual connections without re-entering credentials.
Project Member

Comment 3 by bugdroid1@chromium.org, Jun 14 2018

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

commit f4a3962690d67caaf6ef854fa88dcd244ab1bd8a
Author: Alexander Hendrich <hendrich@chromium.org>
Date: Thu Jun 14 09:00:58 2018

Fix AutoConnectHandler for AllowOnlyPolicyNetworksTo[Auto]Connect

Fix1: If either AllowOnlyPolicyNetworksToConnect or
AllowOnlyPolicyNetworksToAutoconnect is enabled, this CL will
additionally remove the network configurations
(AllowOnlyPolicyNetworksToConnect) or disable auto-connect (for
AllowOnlyPolicyNetworksToAutoconnect) for the unmanaged networks to
prevent Shill from re-connecting when searching for a best service to
connect.

Fix2: Fixed missing disconnect, when AllowOnlyPolicyNetworksToConnect
gets enabled. This disconnect was incorrectly prevented by the
|applied_autoconnect_policy_| boolean before, which ensures that the
disconnect on AllowOnlyPolicyNetworksToAutoconnect only happens once.

Bug:  850550 
Change-Id: I869ed49d58adb82707e8856e896c5a141f6f6625
Reviewed-on: https://chromium-review.googlesource.com/1091051
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Alexander Hendrich <hendrich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567199}
[modify] https://crrev.com/f4a3962690d67caaf6ef854fa88dcd244ab1bd8a/chromeos/network/auto_connect_handler.cc
[modify] https://crrev.com/f4a3962690d67caaf6ef854fa88dcd244ab1bd8a/chromeos/network/auto_connect_handler.h

Status: Fixed (was: Started)

Sign in to add a comment