chromeos networking: move from security to securityclass property

chromeos networking: move from security to securityclass property

Adopt shill's kSecurityClassProperty, in place of shill's
kSecurityProperty. This eliminates the need for Chrome to understand
the equivalence of WPA and RSN networks.

This CL introduces only one functional change. Namely,
NetworkState::GetSpecifier will now collapse WPA and RSN networks into
one class (by virtue of using kSecurityClassProperty). The previous
behavior is considered a bug.

After this CL, only WifiAccessPointInfoProviderChromeOs still uses
kSecurityProperty. In that one case, we want to break out WPA and RSN
networks, to understand what fraction of systems are connecting
through older APs.

While there: remove a couple of unused files in chromeos/test/data/network/policy/.

BUG=chromium:440032
TEST=ManagedNetworkConfigurationHandlerTest.*
TEST=ONC*

Committed: https://crrev.com/9bb8d4e41faf758814fbb1fa89771d6f09a045e9
Cr-Commit-Position: refs/heads/master@{#310840}

[mukesh agrawal, 2014-12-08 21:19:54]

I'm having no luck with the tryservers (maybe a permissions issue?), so I figure
I'll just let you at the CL now.

[mukesh agrawal, 2014-12-08 21:20:13]

quiche@chromium.org changed reviewers:
+ pneubeck@chromium.org, stevenjb@chromium.org

[stevenjb, 2014-12-08 22:27:21]

Assuming that Shill accepts SecurityClass for configuration, and there are no
remaining references to shill::kSecurity in the code, this lgtm, thanks!

https://codereview.chromium.org/788633003/diff/1/chrome/browser/chromeos/opti...
File chrome/browser/chromeos/options/wifi_config_view.cc (right):

https://codereview.chromium.org/788633003/diff/1/chrome/browser/chromeos/opti...
chrome/browser/chromeos/options/wifi_config_view.cc:705:
shill::kSecurityClassProperty, security_class);
Have you verified that we can use SecurityClass to configure a new network in
Shill?

https://codereview.chromium.org/788633003/diff/1/chromeos/network/onc/onc_tra...
File chromeos/network/onc/onc_translator_onc_to_shill.cc (right):

https://codereview.chromium.org/788633003/diff/1/chromeos/network/onc/onc_tra...
chromeos/network/onc/onc_translator_onc_to_shill.cc:199: // XXX check how used
Elim

[mukesh agrawal, 2014-12-16 22:07:00]

Sorry... I forgot to separate out the rebase from the fix. The only file with
actual changes from PS1 to PS2 is onc_translation_onc_to_shill.cc.

https://codereview.chromium.org/788633003/diff/1/chrome/browser/chromeos/opti...
File chrome/browser/chromeos/options/wifi_config_view.cc (right):

https://codereview.chromium.org/788633003/diff/1/chrome/browser/chromeos/opti...
chrome/browser/chromeos/options/wifi_config_view.cc:705:
shill::kSecurityClassProperty, security_class);
On 2014/12/08 22:27:21, stevenjb wrote:
> Have you verified that we can use SecurityClass to configure a new network in
> Shill?

I've updated shill and the wifi autotests. I tested manually as follows:

  for each of {open, wep, psk}:
    1. configure a hidden network using chrome://settings -> Add connection
    2. verify that shill attempts to locate the network
    3. check that the shill profile is updated correctly

Note that step 2 required a fix for crbug.com/397763. But that's unrelated to
the kSecurityProperty -> kSecurityClassProperty transition.

https://codereview.chromium.org/788633003/diff/1/chromeos/network/onc/onc_tra...
File chromeos/network/onc/onc_translator_onc_to_shill.cc (right):

https://codereview.chromium.org/788633003/diff/1/chromeos/network/onc/onc_tra...
chromeos/network/onc/onc_translator_onc_to_shill.cc:199: // XXX check how used
On 2014/12/08 22:27:21, stevenjb wrote:
> Elim

Done.

[mukesh agrawal, 2014-12-16 22:07:07]

The CQ bit was checked by quiche@chromium.org

[commit-bot: I haz the power, 2014-12-16 22:07:43]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/788633003/20001

[commit-bot: I haz the power, 2014-12-16 22:43:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-12-16 22:43:24]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[mukesh agrawal, 2014-12-17 01:23:19]

On 2014/12/16 22:43:24, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   linux_chromium_chromeos_rel_ng on tryserver.chromium.linux
>
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

Lots of test failures in ONCTranslator and ManagedNetworkConfigurationHandler.
I'll take a deeper look tomorrow.

[mukesh agrawal, 2014-12-17 23:26:17]

stevenjb/pneubeck
-----------------
Could one of you take another look? It turns out I'd missed some necessary
changes to the tests. Most of the changes are mechanical, but I'd like someone
to look over:

1) The removal of
ManagedNetworkConfigurationHandlerTest.SetPolicyUpdateManagedEquivalentSecurity
2) The removal of the shill_wifi_wpa1.json case of ONCTranslatorShillToOncTest
3) The removal of
chromeos/test/data/network/policy/shill_policy_on_unmanaged_wifi1_wo_uidata.json
4) The removal of
chromeos/test/data/network/policy/shill_unmanaged_wifi1_wo_uidata.json

oshima
------
Please review the change to athena/system/network_selector.cc.

Thanks.

[mukesh agrawal, 2014-12-17 23:26:31]

quiche@chromium.org changed reviewers:
+ oshima@chromium.org

[mukesh agrawal, 2014-12-17 23:26:31]

quiche@chromium.org changed required reviewers:
+ oshima@chromium.org

[oshima, 2014-12-18 12:02:12]

athena lgtm

[stevenjb (google-dont-use), 2014-12-22 18:44:45]

stevenjb@google.com changed reviewers:
+ stevenjb@google.com

[stevenjb (google-dont-use), 2014-12-22 18:44:45]

LGTM

[mukesh agrawal, 2014-12-22 23:09:49]

quiche@chromium.org changed reviewers:
+ erikwright@chromium.org
- stevenjb@google.com

[mukesh agrawal, 2014-12-22 23:09:49]

quiche@chromium.org changed required reviewers:
+ erikwright@chromium.org

[stevenjb, 2014-12-23 19:26:14]

lgtm

[pneubeck (no reviews), 2015-01-04 17:07:05]

The commit message incorrectly mentions a kSecurityClass instead of
kSecurityProperty.

[pneubeck (no reviews), 2015-01-04 20:47:40]

lgtm with one minor comment

https://codereview.chromium.org/788633003/diff/120001/chromeos/network/shill_...
File chromeos/network/shill_property_util.cc (right):

https://codereview.chromium.org/788633003/diff/120001/chromeos/network/shill_...
chromeos/network/shill_property_util.cc:250: shill::kSecurityClassProperty,
&security_class);
can be replaced by CopyStringFromDictionary

[pneubeck (no reviews), 2015-01-05 16:55:11]

thanks for this simplification!

[erikwright (departed), 2015-01-09 15:00:03]

components/ LGTM

[mukesh agrawal, 2015-01-09 18:20:56]

The CQ bit was checked by quiche@chromium.org

[commit-bot: I haz the power, 2015-01-09 18:22:17]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/788633003/140001

[commit-bot: I haz the power, 2015-01-09 18:28:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-01-09 18:28:09]

Try jobs failed on following builders:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/10...)
  android_aosp on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_aosp/bu...)
  android_arm64_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_arm64_d...)
  android_chromium_gn_compile_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_clang_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  android_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_compile...)
  android_dbg_tests_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_asan_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_compile_dbg_ng on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_rel_ng on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)
  win_chromium_compile_dbg on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_rel_ng on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_ng on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[mukesh agrawal, 2015-01-09 19:28:00]

Conflicts were in
- chromeos/network/network_cert_migrator.cc
- chromeos/network/onc/onc_translator_onc_to_shill.cc

Both conflict resolutions were straight-forward.

[mukesh agrawal, 2015-01-09 19:28:07]

The CQ bit was checked by quiche@chromium.org

[commit-bot: I haz the power, 2015-01-09 19:29:09]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/788633003/160001

[commit-bot: I haz the power, 2015-01-09 20:13:49]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2015-01-09 20:15:20]

Patchset 9 (id:??) landed as
https://crrev.com/9bb8d4e41faf758814fbb1fa89771d6f09a045e9
Cr-Commit-Position: refs/heads/master@{#310840}