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

Issue 848719 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 6
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Feature



Sign in to add a comment

Enforce 'AllowOnlyPolicyNetworksToConnect' policy only if there are connectable policy networks available

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

Issue description

broken out of  http://crbug.com/837205 

We now have several methods in ONC to prevent users from connecting to unmanaged networks. We can either block specific SSIDs using the 'AllowToConnect' property or we can disallow all unanaged networks using the global 'AllowOnlyPolicyNetworksToConnect' property.

However, we want to force users to use managed networks if available, but also let them connect to unmanaged networks if no managed network is available (e.g. at home). That means to only enforce the 'AllowOnlyPolicyNetworksToConnect' policy, if there is a connectable managed wifi network available.
 
Cc: benchan@chromium.org
I worry that this would break existing valid use cases for AllowOnlyPolicyNetworksToConnect, e.g. for work devices that are not intended to be used elsewhere.

It is also potentially fragile for situations where a policy network is on the edge of its range. We also do not scan constantly, so if a device is connected to a non policy network, and moved to a location where the policy network is available, the policy network might not show up in the initial scan causing confusing behavior.

We already prefer more secure networks, and I believe we also prefer policy networks? (If not that seems like a reasonable change).

Other possible options to help prevent users from connecting to non policy networks when at the enterprise environment:
* Add a policy to disable connecting to open networks (this requires users to have a secure home network, but for a policy controlled device that seems like a reasonable requirement).
* Add a policy to disable scanning for networks. New networks could only be added and remembered using a known (typed in) SSID.

Components: OS>Systems>Network
I totally agree with you points/suggestions.

I think this feature is ment as an additional feature (like AllowOnlyPolicyNetworksToConnectIfAvailable), instead of replacing the already existing (AllowOnlyPolicyNetworksToConnect) feature's functionallity. It would probably make sense to have only one both of active at the same time.
Implementing this feature in naive way (connected to unmanaged network & see managed network -> disconnect and request ConnectToBestService (managed network should be listed above the unmanaged network)) should be straightforward, however we don't realy know how to handle these two scenarios yet:

1) Managed network has AutoConnect=false
Do we disconnect and not auto-connect to the managed network, i.e. leaving the user without a connection? Do we only enforce this policy for managed auto-connect networks?

2) Connection to managed network fails
We could observe failed connection requests and ignore the managed network for now. But how do we enforce the policy again if the network becomes connectable again?
We could only enforce this policy if the managed network has a minimum signal strength, but that doesn't guarantuee a successful connection either.


Concerning not scanning constantly:
Can the user manually disable network scans? Is there a fixed time interval for repeating network scans? Or do we stop scanning if we found a stable connection?
I don't see a big problem with not scanning constantly as long as the user can't manually disable network scans. If the user manages to move his device into the range of a managed network without triggering a network scan, then we still allow him to stay on the unmanaged network. I feel like this policy is more of a "we prefer to use managed networks over unmanaged networks" than a "unmanaged networks are strictly forbidden".
For 1), makes sense to only disconnect if there's an autoconnect managed network available.

For 2) I think failing the connection and remaining disconnected is OK, as long as that network is visible.

1) That's a weird edge case, I'm not too concerned with how e decide to handle it.

2) This is the sort of scenario that concerns me. If a managed network is flaky (e.g. on the edge of its range or just flaky hardware) and other networks are available, it's likely going to be a frustrating / confusing situation however we handle this.

WRT scanning, to the best of my knowledge:
a) Scanning occurs on start up and awake from sleep.
b) Opening any network UI explicitly triggers a scan, and rerequests a scan every 10 seconds.
c) There is no setting affecting scanning (and we really don't want one).

If a user with this policy is connected to a non policy network, we could trigger scans periodically, but that will impact batter life so we should be careful about that.

Project Member

Comment 7 by bugdroid1@chromium.org, Jul 2

The following revision refers to this bug:
  https://chromium.googlesource.com/aosp/platform/system/connectivity/shill/+/044145ef409b9617e16966c8cd372d7a9c08d4ae

commit 044145ef409b9617e16966c8cd372d7a9c08d4ae
Author: Alexander Hendrich <hendrich@chromium.org>
Date: Mon Jul 02 18:30:12 2018

shill: Sort managed networks above unmanaged networks

The |managed_credentials_| was previously entangled with the
|has_ever_connected_| property. This caused a previously connected
network to be treated just like a managed network. Splitting these two
properties in the compare method ensures that managed networks are
preferred over unmanaged networks.

BUG= chromium:848719 
TEST=shill unit test ServiceTest.Compare

Change-Id: I23cfef784bd34d3eca2b95cffc0724fb0085c3c0
Reviewed-on: https://chromium-review.googlesource.com/1120255
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Alexander Hendrich <hendrich@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Ben Chan <benchan@chromium.org>

[modify] https://crrev.com/044145ef409b9617e16966c8cd372d7a9c08d4ae/service.cc
[modify] https://crrev.com/044145ef409b9617e16966c8cd372d7a9c08d4ae/service.h
[modify] https://crrev.com/044145ef409b9617e16966c8cd372d7a9c08d4ae/service_unittest.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Aug 1

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

commit e4d3b26dabf131705d0f96614a01bdb850edbf07
Author: Alexander Hendrich <hendrich@chromium.org>
Date: Wed Aug 01 11:50:54 2018

Add 'AllowOnlyPolicyNetworksToConnectIfAvailable' policy to ONC

Added the 'AllowOnlyPolicyNetworksToConnectIfAvailable' policy to ONC.
If a managed network is available in the lsit of visible networks, all
unmanaged networks are considered as prohibited. However, we don't
remove network configurations for unmanaged networks because we might
want to re-connect with them, once we lose availability of the managed
network.
If this policy is enabled, we are currently connected to an unmanaged
network and a managed network is available, we automatically connect
to the available managed network when the network scan finishes.

Bug:  848719 
Change-Id: I6da37d1b9d7813ff7bdfd52ab0e28081f9e48631
Reviewed-on: https://chromium-review.googlesource.com/1122979
Commit-Queue: Alexander Hendrich <hendrich@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Cait Phillips <caitkp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579769}
[modify] https://crrev.com/e4d3b26dabf131705d0f96614a01bdb850edbf07/chromeos/network/auto_connect_handler.cc
[modify] https://crrev.com/e4d3b26dabf131705d0f96614a01bdb850edbf07/chromeos/network/auto_connect_handler_unittest.cc
[modify] https://crrev.com/e4d3b26dabf131705d0f96614a01bdb850edbf07/chromeos/network/device_state.cc
[modify] https://crrev.com/e4d3b26dabf131705d0f96614a01bdb850edbf07/chromeos/network/device_state.h
[modify] https://crrev.com/e4d3b26dabf131705d0f96614a01bdb850edbf07/chromeos/network/managed_network_configuration_handler.h
[modify] https://crrev.com/e4d3b26dabf131705d0f96614a01bdb850edbf07/chromeos/network/managed_network_configuration_handler_impl.cc
[modify] https://crrev.com/e4d3b26dabf131705d0f96614a01bdb850edbf07/chromeos/network/managed_network_configuration_handler_impl.h
[modify] https://crrev.com/e4d3b26dabf131705d0f96614a01bdb850edbf07/chromeos/network/managed_network_configuration_handler_unittest.cc
[modify] https://crrev.com/e4d3b26dabf131705d0f96614a01bdb850edbf07/chromeos/network/mock_managed_network_configuration_handler.h
[modify] https://crrev.com/e4d3b26dabf131705d0f96614a01bdb850edbf07/chromeos/network/mock_network_state_handler.h
[modify] https://crrev.com/e4d3b26dabf131705d0f96614a01bdb850edbf07/chromeos/network/network_connection_handler_impl.cc
[modify] https://crrev.com/e4d3b26dabf131705d0f96614a01bdb850edbf07/chromeos/network/network_state_handler.cc
[modify] https://crrev.com/e4d3b26dabf131705d0f96614a01bdb850edbf07/chromeos/network/network_state_handler.h
[modify] https://crrev.com/e4d3b26dabf131705d0f96614a01bdb850edbf07/chromeos/network/network_state_handler_unittest.cc
[modify] https://crrev.com/e4d3b26dabf131705d0f96614a01bdb850edbf07/chromeos/network/onc/onc_signature.cc
[modify] https://crrev.com/e4d3b26dabf131705d0f96614a01bdb850edbf07/chromeos/network/onc/onc_validator.cc
[add] https://crrev.com/e4d3b26dabf131705d0f96614a01bdb850edbf07/chromeos/test/data/network/policy/policy_allow_only_policy_networks_to_connect_if_available.onc
[modify] https://crrev.com/e4d3b26dabf131705d0f96614a01bdb850edbf07/components/onc/onc_constants.cc
[modify] https://crrev.com/e4d3b26dabf131705d0f96614a01bdb850edbf07/components/onc/onc_constants.h
[modify] https://crrev.com/e4d3b26dabf131705d0f96614a01bdb850edbf07/components/sync_wifi/wifi_config_delegate_chromeos_unittest.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Aug 2

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

commit 7ac3c3c26641f436b6d8988f3aeb3fa07d7fe667
Author: Alexander Hendrich <hendrich@chromium.org>
Date: Thu Aug 02 08:50:40 2018

Add 'AllowOnlyPolicyNetworksToConnectIfAvailable' policy to ONC [UI]

This CL implements the UI changes for the new
AllowOnlyPolicyNetworksToConnectIfAvailable policy in chrome settings.

Bug:  848719 
Cq-Include-Trybots: luci.chromium.try:closure_compilation
Change-Id: I41c9b935f291e2cdfb95c6ef52b216b2a268af43
Reviewed-on: https://chromium-review.googlesource.com/1124448
Commit-Queue: Alexander Hendrich <hendrich@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580116}
[modify] https://crrev.com/7ac3c3c26641f436b6d8988f3aeb3fa07d7fe667/chrome/browser/resources/settings/internet_page/internet_detail_page.html
[modify] https://crrev.com/7ac3c3c26641f436b6d8988f3aeb3fa07d7fe667/chrome/browser/resources/settings/internet_page/internet_detail_page.js
[modify] https://crrev.com/7ac3c3c26641f436b6d8988f3aeb3fa07d7fe667/chrome/browser/resources/settings/internet_page/internet_page.html
[modify] https://crrev.com/7ac3c3c26641f436b6d8988f3aeb3fa07d7fe667/chrome/browser/resources/settings/internet_page/internet_page.js
[modify] https://crrev.com/7ac3c3c26641f436b6d8988f3aeb3fa07d7fe667/chrome/browser/resources/settings/internet_page/internet_subpage.js
[modify] https://crrev.com/7ac3c3c26641f436b6d8988f3aeb3fa07d7fe667/chrome/test/data/extensions/api_test/networking_private/chromeos/test.js
[modify] https://crrev.com/7ac3c3c26641f436b6d8988f3aeb3fa07d7fe667/extensions/browser/api/networking_private/networking_private_chromeos.cc
[modify] https://crrev.com/7ac3c3c26641f436b6d8988f3aeb3fa07d7fe667/extensions/common/api/networking_onc.idl
[modify] https://crrev.com/7ac3c3c26641f436b6d8988f3aeb3fa07d7fe667/extensions/common/api/networking_private.idl
[modify] https://crrev.com/7ac3c3c26641f436b6d8988f3aeb3fa07d7fe667/third_party/closure_compiler/externs/networking_private.js

Project Member

Comment 10 by bugdroid1@chromium.org, Aug 4

The following revision refers to this bug:
  https://chromium.googlesource.com/aosp/platform/system/connectivity/shill/+/3c68af2465d7d4aaaf658d43382108c07484d751

commit 3c68af2465d7d4aaaf658d43382108c07484d751
Author: Alexander Hendrich <hendrich@chromium.org>
Date: Sat Aug 04 01:43:59 2018

shill: Fix network sorting order

Move technology and priority above managed_credentials in
Service::Compare to allow ethernet or preferred (priority=1) networks to
be sorted above managed networks.
This CL also removes the priority_within_technology property since it is
unused and obsolete due to the new ordering of sorting criterias.

BUG= chromium:848719 
TEST=shill unit test ServiceTest.Compare

Change-Id: Ia9e2fcf7270f098f9a8215a42c1c04f737a0a1f4
Reviewed-on: https://chromium-review.googlesource.com/1136437
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Alexander Hendrich <hendrich@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Ben Chan <benchan@chromium.org>

[modify] https://crrev.com/3c68af2465d7d4aaaf658d43382108c07484d751/manager_unittest.cc
[modify] https://crrev.com/3c68af2465d7d4aaaf658d43382108c07484d751/doc/service-api.txt
[modify] https://crrev.com/3c68af2465d7d4aaaf658d43382108c07484d751/service.cc
[modify] https://crrev.com/3c68af2465d7d4aaaf658d43382108c07484d751/service.h
[modify] https://crrev.com/3c68af2465d7d4aaaf658d43382108c07484d751/service_unittest.cc

Owner: hendrich@chromium.org
Status: Fixed (was: Untriaged)
Project Member

Comment 12 by bugdroid1@chromium.org, Aug 7

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/system_api/+/245db4e4e96e90647430812a628006fef6b0b016

commit 245db4e4e96e90647430812a628006fef6b0b016
Author: Alexander Hendrich <hendrich@chromium.org>
Date: Tue Aug 07 08:51:15 2018

shill: Remove PriorityWithinTechnology property

The PriorityWithinTechnology property is unused and also obsolete due to
the recent changes in CL:1136437.

CQ-DEPEND=CL:1136437
BUG= chromium:848719 
TEST=none

Change-Id: I3f439fd3dd0a1f0fc4e0bd4c27b4d911ff578101
Reviewed-on: https://chromium-review.googlesource.com/1150233
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Alexander Hendrich <hendrich@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Ben Chan <benchan@chromium.org>

[modify] https://crrev.com/245db4e4e96e90647430812a628006fef6b0b016/dbus/shill/dbus-constants.h

Project Member

Comment 13 by bugdroid1@chromium.org, Sep 25

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

commit 36435cb6d3e7e81cc696438d93bebe6b73d6c770
Author: Alexander Hendrich <hendrich@chromium.org>
Date: Tue Sep 25 17:18:47 2018

Update ONC documentation (GlobelNetworkConfiguration & Recommended)

This CL updates the ONC documentation and adds a section + examples for
GlobalNetworkConfiguration policies
(AllowOnlyPolicyNetworksToAutoconnect, AllowOnlyPolicyNetworksToConnect,
AllowOnlyPolicyNetworksToConnectIfAvailable, BlacklistedHexSSIDs,
DisableNetworkTypes) and recommended values.

Bug:  426390 ,  837205 ,  280146 ,  208378 ,  848719 
Change-Id: I9b573f4dd8533b8d7fe83da2e7e5e45e2eb18573
Reviewed-on: https://chromium-review.googlesource.com/1238584
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Alexander Hendrich <hendrich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593977}
[modify] https://crrev.com/36435cb6d3e7e81cc696438d93bebe6b73d6c770/components/onc/docs/onc_spec.md

Sign in to add a comment