Ensure test coverage for handling key rotation for different policy types |
||
Issue descriptionAs per issue 663870 , the policy test server didn't perform key rotations previously despite its documentation said otherwise. This means that there are possible gaps in test coverage of handling the key rotations. In particular, user, device and device-local account policy for Chrome OS and user policy for desktop have to be checked.
,
Dec 6 2016
Looks like the test coverage is not very well with regards to end-to-end testing of the whole policy stack. The first probable issue has been discovered: issue 671659 .
,
Mar 3 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6581fa97d9ac35a9c541856a0acc04c8be2cfc20 commit 6581fa97d9ac35a9c541856a0acc04c8be2cfc20 Author: emaxx <emaxx@chromium.org> Date: Fri Mar 03 03:54:13 2017 Fix handling of device cloud signing policy key rotation This CL fixes issues with handling of the signing key rotation for the device policy. There were some issues with not loading the updated owner key right after the new policy is stored. Even though the key would eventually be loaded thanks to the OwnerKeySet signal emitted by the session manager, there may be some window during which the old key is still used on the Chrome side. Also the CL adds a browser end-to-end test for browser policy key rotation (using a local policy test server). Some fake testing-only stubs were also fixed in order to support changing the signing keys (similar to how this is handled by the session manager). BUG= 671659 ,668716 TEST=new browser test Review-Url: https://codereview.chromium.org/2558543003 Cr-Commit-Position: refs/heads/master@{#454506} [modify] https://crrev.com/6581fa97d9ac35a9c541856a0acc04c8be2cfc20/chrome/browser/chromeos/policy/device_cloud_policy_browsertest.cc [modify] https://crrev.com/6581fa97d9ac35a9c541856a0acc04c8be2cfc20/chrome/browser/chromeos/settings/session_manager_operation.cc [modify] https://crrev.com/6581fa97d9ac35a9c541856a0acc04c8be2cfc20/chromeos/BUILD.gn [modify] https://crrev.com/6581fa97d9ac35a9c541856a0acc04c8be2cfc20/chromeos/dbus/fake_session_manager_client.cc
,
Mar 3 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3fa1b666a54ad027289ec0c1752fcfb6d138b512 commit 3fa1b666a54ad027289ec0c1752fcfb6d138b512 Author: alph <alph@chromium.org> Date: Fri Mar 03 07:59:21 2017 Revert of Fix handling of device cloud signing policy key rotation (patchset #8 id:140001 of https://codereview.chromium.org/2558543003/ ) Reason for revert: Broke KeyRotationDeviceCloudPolicyTest.Basic test https://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%20Tests%20%281%29/builds/34579 Original issue's description: > Fix handling of device cloud signing policy key rotation > > This CL fixes issues with handling of the signing key rotation for the > device policy. There were some issues with not loading the updated > owner key right after the new policy is stored. Even though the key > would eventually be loaded thanks to the OwnerKeySet signal emitted > by the session manager, there may be some window during which the old > key is still used on the Chrome side. > > Also the CL adds a browser end-to-end test for browser policy key > rotation (using a local policy test server). Some fake testing-only > stubs were also fixed in order to support changing the signing keys > (similar to how this is handled by the session manager). > > BUG= 671659 ,668716 > TEST=new browser test > > Review-Url: https://codereview.chromium.org/2558543003 > Cr-Commit-Position: refs/heads/master@{#454506} > Committed: https://chromium.googlesource.com/chromium/src/+/6581fa97d9ac35a9c541856a0acc04c8be2cfc20 TBR=atwilson@chromium.org,mnissler@chromium.org,stevenjb@chromium.org,emaxx@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 671659 ,668716 Review-Url: https://codereview.chromium.org/2727863004 Cr-Commit-Position: refs/heads/master@{#454541} [modify] https://crrev.com/3fa1b666a54ad027289ec0c1752fcfb6d138b512/chrome/browser/chromeos/policy/device_cloud_policy_browsertest.cc [modify] https://crrev.com/3fa1b666a54ad027289ec0c1752fcfb6d138b512/chrome/browser/chromeos/settings/session_manager_operation.cc [modify] https://crrev.com/3fa1b666a54ad027289ec0c1752fcfb6d138b512/chromeos/BUILD.gn [modify] https://crrev.com/3fa1b666a54ad027289ec0c1752fcfb6d138b512/chromeos/dbus/fake_session_manager_client.cc
,
Mar 7 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8aaab6cd51f40e6ca50c01423f64c49d5bbb150f commit 8aaab6cd51f40e6ca50c01423f64c49d5bbb150f Author: emaxx <emaxx@chromium.org> Date: Tue Mar 07 14:57:33 2017 Reland Fix handling of device cloud signing policy key rotation The original CL (crrev.com/2558543003) was reverted due to the failure of the added test KeyRotationDeviceCloudPolicyTest.Basic. This CL fixes this test by fixing the incorrect usage of DCHECK in the fake class for testing: the DCHECK erroneously contained an actual piece of logic. Original CL's description: > This CL fixes issues with handling of the signing key rotation for the > device policy. There were some issues with not loading the updated > owner key right after the new policy is stored. Even though the key > would eventually be loaded thanks to the OwnerKeySet signal emitted > by the session manager, there may be some window during which the old > key is still used on the Chrome side. > > Also the CL adds a browser end-to-end test for browser policy key > rotation (using a local policy test server). Some fake testing-only > stubs were also fixed in order to support changing the signing keys > (similar to how this is handled by the session manager). > > BUG= 671659 , 668716 > TEST=new browser test > > Review-Url: https://codereview.chromium.org/2558543003 > Cr-Commit-Position: refs/heads/master@{#454506} > Committed: https://chromium.googlesource.com/chromium/src/+/6581fa97d9ac35a9c541856a0acc04c8be2cfc20 BUG= 671659 , 668716 TEST=new browser test TBR=stevenjb@chromium.org, atwilson@chromium.org TBR_REASON=The CL is almost identical to the one that was already reviewed, except for fixing a small stupid bug in the test. Review-Url: https://codereview.chromium.org/2737483006 Cr-Commit-Position: refs/heads/master@{#455075} [modify] https://crrev.com/8aaab6cd51f40e6ca50c01423f64c49d5bbb150f/chrome/browser/chromeos/policy/device_cloud_policy_browsertest.cc [modify] https://crrev.com/8aaab6cd51f40e6ca50c01423f64c49d5bbb150f/chrome/browser/chromeos/settings/session_manager_operation.cc [modify] https://crrev.com/8aaab6cd51f40e6ca50c01423f64c49d5bbb150f/chromeos/BUILD.gn [modify] https://crrev.com/8aaab6cd51f40e6ca50c01423f64c49d5bbb150f/chromeos/dbus/fake_session_manager_client.cc |
||
►
Sign in to add a comment |
||
Comment 1 by emaxx@chromium.org
, Nov 25 2016