Fix handling of device cloud signing policy key rotation

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

[emaxx, 2016-12-06 19:11:38]

The CQ bit was checked by emaxx@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-06 19:12:09]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-06 19:17:54]

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

[commit-bot: I haz the power, 2016-12-06 19:17:55]

Dry run: Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)

[emaxx, 2016-12-06 21:05:26]

The CQ bit was checked by emaxx@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-06 21:06:08]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[emaxx, 2016-12-06 21:58:37]

emaxx@chromium.org changed reviewers:
+ atwilson@chromium.org

[emaxx, 2016-12-06 21:58:38]

Drew, PTAL.

[commit-bot: I haz the power, 2016-12-06 22:17:10]

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

[commit-bot: I haz the power, 2016-12-06 22:17:11]

Dry run: This issue passed the CQ dry run.

[emaxx, 2016-12-07 16:23:05]

The CQ bit was checked by emaxx@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-07 16:23:31]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[emaxx, 2016-12-07 16:26:58]

Description was changed from

==========
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 various issues around not loading the updated
owner key right after the new policy is stored.

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
==========

to

==========
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.

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
==========

[emaxx, 2016-12-07 16:31:29]

Mattias, CC'ing you in case you would have interest to take a look.

[commit-bot: I haz the power, 2016-12-07 18:01:19]

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

[commit-bot: I haz the power, 2016-12-07 18:01:19]

Dry run: This issue passed the CQ dry run.

[Mattias Nissler (ping if slow), 2016-12-08 12:32:20]

mnissler@chromium.org changed reviewers:
+ mnissler@chromium.org

[Mattias Nissler (ping if slow), 2016-12-08 12:32:21]

Haven't done a thorough review, but looks great at a high level. Thanks for
fixing this!

https://codereview.chromium.org/2558543003/diff/40001/chrome/browser/chromeos...
File chrome/browser/chromeos/settings/session_manager_operation.cc (right):

https://codereview.chromium.org/2558543003/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/settings/session_manager_operation.cc:238:
force_key_load_ = true;
This change is good as it'll make sure won't race. FWIW, I'd still advise to
keep the OWNER_KEY_SET handling intact (which you do), since for some cases keys
are generated and installed outside Chrome (e.g. consumer ownership).

[emaxx, 2016-12-08 13:35:40]

Description was changed from

==========
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.

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
==========

to

==========
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
==========

[emaxx, 2017-02-13 17:48:00]

The CQ bit was checked by emaxx@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-13 17:48:53]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-13 18:46:00]

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

[commit-bot: I haz the power, 2017-02-13 18:46:01]

Dry run: This issue passed the CQ dry run.

[emaxx, 2017-02-22 16:59:30]

Drew, ping.
This is not high-priority as this shouldn't hit any real life usages (unless we
decide to start the key rotation on the server side). But still worth to fix the
bug, and to prevent the new code from duplicating this bug.

[Andrew T Wilson (Slow), 2017-02-23 12:20:38]

LGTM

https://codereview.chromium.org/2558543003/diff/60001/chrome/browser/chromeos...
File chrome/browser/chromeos/policy/device_cloud_policy_browsertest.cc (right):

https://codereview.chromium.org/2558543003/diff/60001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/device_cloud_policy_browsertest.cc:224:
std::unique_ptr<int> awaited_policy_value_;
hrm, not clear why this is a unique_ptr instead of just an int?

[emaxx, 2017-02-23 15:32:35]

https://codereview.chromium.org/2558543003/diff/60001/chrome/browser/chromeos...
File chrome/browser/chromeos/policy/device_cloud_policy_browsertest.cc (right):

https://codereview.chromium.org/2558543003/diff/60001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/device_cloud_policy_browsertest.cc:224:
std::unique_ptr<int> awaited_policy_value_;
On 2017/02/23 12:20:38, Andrew T Wilson (Slow) wrote:
> hrm, not clear why this is a unique_ptr instead of just an int?

I just prefer to use the semantics of std::optional in favor of assigning fake
values to variables. But, yes, probably that's just an unnecessary
complication...

Done, replaced this with a simple int.

[emaxx, 2017-02-23 15:32:39]

The CQ bit was checked by emaxx@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-23 15:32:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[emaxx, 2017-02-23 15:33:59]

emaxx@chromium.org changed reviewers:
+ stevenjb@chromium.org

[emaxx, 2017-02-23 15:33:59]

stevenjb@: PTAL at changes under chromeos/.

[Andrew T Wilson (Slow), 2017-02-23 15:51:20]

https://codereview.chromium.org/2558543003/diff/60001/chrome/browser/chromeos...
File chrome/browser/chromeos/policy/device_cloud_policy_browsertest.cc (right):

https://codereview.chromium.org/2558543003/diff/60001/chrome/browser/chromeos...
chrome/browser/chromeos/policy/device_cloud_policy_browsertest.cc:224:
std::unique_ptr<int> awaited_policy_value_;
On 2017/02/23 15:32:34, emaxx wrote:
> On 2017/02/23 12:20:38, Andrew T Wilson (Slow) wrote:
> > hrm, not clear why this is a unique_ptr instead of just an int?
> 
> I just prefer to use the semantics of std::optional in favor of assigning fake
> values to variables. But, yes, probably that's just an unnecessary
> complication...
> 
> Done, replaced this with a simple int.

Sure, my thought was just that if you don't have awaited_policy_value_ set, then
TestPolicyChangedCallback will never exit the runloop anyway so it seems like
you can just always assume it's set at the point you check it - if it's not,
your test won't exit?

[commit-bot: I haz the power, 2017-02-23 16:48:33]

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

[commit-bot: I haz the power, 2017-02-23 16:48:34]

Dry run: This issue passed the CQ dry run.

[stevenjb, 2017-02-23 18:14:28]

https://codereview.chromium.org/2558543003/diff/80001/chromeos/dbus/fake_sess...
File chromeos/dbus/fake_session_manager_client.cc (right):

https://codereview.chromium.org/2558543003/diff/80001/chromeos/dbus/fake_sess...
chromeos/dbus/fake_session_manager_client.cc:18: #include
"components/policy/proto/device_management_backend.pb.h"
I'm not super keen on adding this dependency, small as it may be. If this is
just for a test, can we have it subclass FakeSessionManagerCLient and implement
this there instead? Or maybe use an observer and  device_policy() to do any
validation?

[emaxx, 2017-02-23 19:20:42]

https://codereview.chromium.org/2558543003/diff/80001/chromeos/dbus/fake_sess...
File chromeos/dbus/fake_session_manager_client.cc (right):

https://codereview.chromium.org/2558543003/diff/80001/chromeos/dbus/fake_sess...
chromeos/dbus/fake_session_manager_client.cc:18: #include
"components/policy/proto/device_management_backend.pb.h"
On 2017/02/23 18:14:28, stevenjb wrote:
> I'm not super keen on adding this dependency, small as it may be. If this is
> just for a test, can we have it subclass FakeSessionManagerCLient and
implement
> this there instead? Or maybe use an observer and  device_policy() to do any
> validation?
> 

Hmm, I understand that having less dependencies is always better.

But the whole FakeSessionManagerClient class is dedicated for tests, so I don't
really get what conceptual difference would be there between these two fake
client implementations.

Also note that the new functionality that I add here to FakeSessionManagerClient
(storing the owner key in the file) is effectively just closing the gap between
the API contract of the real session manager and this FakeSessionManagerClient.
The consumer code in the policy stack expects the key to be stored in that file,
and IMO it was an oversight that no test actually covers this step.


But if you insist on getting rid of this dependency, then maybe the same can be
achieved by adding an observer or a callback injector into
FakeSessionManagerClient...

[stevenjb, 2017-02-23 19:27:34]

Actually, the Fake implementations exist largely to allow Chrome OS to run
on Linux. We sometimes leverage them in browser_tests as well. In general
they should not do more than the real implementation does (i.e. should not
have additional dependencies).

I am fine with expanding the fake to store additional data, but we should
avoid additional dependencies if at all possible. I would prefer adding a
Delegate or callback to the fake. (Actually, it looks like
SessionManagerClient already has a StubDelegate).


On Thu, Feb 23, 2017 at 12:20 PM, <emaxx@chromium.org> wrote:

>
> https://codereview.chromium.org/2558543003/diff/80001/
> chromeos/dbus/fake_session_manager_client.cc
> File chromeos/dbus/fake_session_manager_client.cc (right):
>
> https://codereview.chromium.org/2558543003/diff/80001/
> chromeos/dbus/fake_session_manager_client.cc#newcode18
> chromeos/dbus/fake_session_manager_client.cc:18: #include
> "components/policy/proto/device_management_backend.pb.h"
> On 2017/02/23 18:14:28, stevenjb wrote:
> > I'm not super keen on adding this dependency, small as it may be. If
> this is
> > just for a test, can we have it subclass FakeSessionManagerCLient and
> implement
> > this there instead? Or maybe use an observer and device_policy() to
> do any
> > validation?
> >
>
> Hmm, I understand that having less dependencies is always better.
>
> But the whole FakeSessionManagerClient class is dedicated for tests, so
> I don't really get what conceptual difference would be there between
> these two fake client implementations.
>
> Also note that the new functionality that I add here to
> FakeSessionManagerClient (storing the owner key in the file) is
> effectively just closing the gap between the API contract of the real
> session manager and this FakeSessionManagerClient.
> The consumer code in the policy stack expects the key to be stored in
> that file, and IMO it was an oversight that no test actually covers this
> step.
>
>
> But if you insist on getting rid of this dependency, then maybe the same
> can be achieved by adding an observer or a callback injector into
> FakeSessionManagerClient...
>
> https://codereview.chromium.org/2558543003/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[emaxx, 2017-03-02 22:06:05]

Steven,

On 2017/02/23 19:27:34, stevenjb wrote:
> Actually, the Fake implementations exist largely to allow Chrome OS to run
> on Linux. We sometimes leverage them in browser_tests as well. In general
> they should not do more than the real implementation does (i.e. should not
> have additional dependencies).
> 
> I am fine with expanding the fake to store additional data, but we should
> avoid additional dependencies if at all possible. I would prefer adding a
> Delegate or callback to the fake. (Actually, it looks like
> SessionManagerClient already has a StubDelegate).

Sorry for returning to this topic again, but I think there are two concepts
mixed up: the fake implementation and the stub implementation.

FakeSessionManagerClient, AFAICS, is used only in unit and browser tests.
And SessionManagerClientStubImpl is what is used when running on Linux.
These two don't have anything in common, except for the base class.

SessionManagerClient::StubDelegate, BTW, is currently purely for the stub
implementation, and I don't think it would be correct to use it in
FakeSessionManagerClient.

Also, actually, SessionManagerClientStubImpl DOES already contain the
functionality I was adding here into FakeSessionManagerClient - see the
implementation of the SessionManagerClientStubImpl::StoreDevicePolicy
method:
https://cs.chromium.org/chromium/src/chromeos/dbus/session_manager_client.cc?...

So I'm not adding anything really new in this CL - it's basically filling the
gap in the fake implementation.
WDYT?

(In case you still request moving the new code out into a delegate -
should the same be done for all similar operations in
SessionManagerClientStubImpl then?)

[stevenjb, 2017-03-02 22:14:14]

On 2017/03/02 22:06:05, emaxx wrote:
> Steven,
> 
> On 2017/02/23 19:27:34, stevenjb wrote:
> > Actually, the Fake implementations exist largely to allow Chrome OS to run
> > on Linux. We sometimes leverage them in browser_tests as well. In general
> > they should not do more than the real implementation does (i.e. should not
> > have additional dependencies).
> > 
> > I am fine with expanding the fake to store additional data, but we should
> > avoid additional dependencies if at all possible. I would prefer adding a
> > Delegate or callback to the fake. (Actually, it looks like
> > SessionManagerClient already has a StubDelegate).
> 
> Sorry for returning to this topic again, but I think there are two concepts
> mixed up: the fake implementation and the stub implementation.
> 
> FakeSessionManagerClient, AFAICS, is used only in unit and browser tests.
> And SessionManagerClientStubImpl is what is used when running on Linux.
> These two don't have anything in common, except for the base class.
> 
> SessionManagerClient::StubDelegate, BTW, is currently purely for the stub
> implementation, and I don't think it would be correct to use it in
> FakeSessionManagerClient.
> 
> Also, actually, SessionManagerClientStubImpl DOES already contain the
> functionality I was adding here into FakeSessionManagerClient - see the
> implementation of the SessionManagerClientStubImpl::StoreDevicePolicy
> method:
>
https://cs.chromium.org/chromium/src/chromeos/dbus/session_manager_client.cc?...
> 
> So I'm not adding anything really new in this CL - it's basically filling the
> gap in the fake implementation.
> WDYT?
> 
> (In case you still request moving the new code out into a delegate -
> should the same be done for all similar operations in
> SessionManagerClientStubImpl then?)

Ugh. I don't know how we ended up with both a stub and a fake, that is
unfortunate.

I forgot what the original issues were, so I will re-review this (and probably
repeat myself), but I will keep the above in mind.

[stevenjb, 2017-03-02 22:23:43]

https://codereview.chromium.org/2558543003/diff/100001/chromeos/dbus/fake_ses...
File chromeos/dbus/fake_session_manager_client.cc (right):

https://codereview.chromium.org/2558543003/diff/100001/chromeos/dbus/fake_ses...
chromeos/dbus/fake_session_manager_client.cc:23: 
Can you explain in a comment why we need to actually write to a file on disk in
a fake implementation that is apparently only used in tests? e.g. is it read
from disk somewhere else?

[emaxx, 2017-03-02 22:46:26]

The CQ bit was checked by emaxx@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-02 22:46:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[emaxx, 2017-03-02 22:47:13]

https://codereview.chromium.org/2558543003/diff/100001/chromeos/dbus/fake_ses...
File chromeos/dbus/fake_session_manager_client.cc (right):

https://codereview.chromium.org/2558543003/diff/100001/chromeos/dbus/fake_ses...
chromeos/dbus/fake_session_manager_client.cc:23: 
On 2017/03/02 22:23:43, stevenjb wrote:
> Can you explain in a comment why we need to actually write to a file on disk
in
> a fake implementation that is apparently only used in tests? e.g. is it read
> from disk somewhere else?
>  

Done, PTAL.

[stevenjb, 2017-03-02 23:10:16]

crhomeos/dbus lgtm

It would be wonderful if someone were to eliminate the StubImpl and integrate it
with the Fake so that we have a single non-cros solution that works for both
Linux development and integration tests.

https://codereview.chromium.org/2558543003/diff/120001/chromeos/dbus/fake_ses...
File chromeos/dbus/fake_session_manager_client.cc (right):

https://codereview.chromium.org/2558543003/diff/120001/chromeos/dbus/fake_ses...
chromeos/dbus/fake_session_manager_client.cc:27: // manages the owner key file
on Chrome OS.
s/manages/that would be managing/

[commit-bot: I haz the power, 2017-03-02 23:53:17]

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

[commit-bot: I haz the power, 2017-03-02 23:53:18]

Dry run: This issue passed the CQ dry run.

[emaxx, 2017-03-03 02:57:44]

On 2017/03/02 23:10:16, stevenjb wrote:
> It would be wonderful if someone were to eliminate the StubImpl and integrate
it
> with the Fake so that we have a single non-cros solution that works for both
> Linux development and integration tests.

Filed crbug.com/698126 for tracking this. I may probably look into this once I
have spare cycles.

[emaxx, 2017-03-03 02:57:57]

https://codereview.chromium.org/2558543003/diff/120001/chromeos/dbus/fake_ses...
File chromeos/dbus/fake_session_manager_client.cc (right):

https://codereview.chromium.org/2558543003/diff/120001/chromeos/dbus/fake_ses...
chromeos/dbus/fake_session_manager_client.cc:27: // manages the owner key file
on Chrome OS.
On 2017/03/02 23:10:16, stevenjb wrote:
> s/manages/that would be managing/

Done.

[emaxx, 2017-03-03 02:58:04]

The CQ bit was checked by emaxx@chromium.org

[emaxx, 2017-03-03 02:58:04]

The patchset sent to the CQ was uploaded after l-g-t-m from
atwilson@chromium.org, stevenjb@chromium.org
Link to the patchset: https://codereview.chromium.org/2558543003/#ps140001
(title: "Comment update")

[commit-bot: I haz the power, 2017-03-03 02:58:23]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-03 03:54:16]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1488509884046670,
"parent_rev": "480806b6e5d48fb4dd96e1e0600b42c181740738", "commit_rev":
"6581fa97d9ac35a9c541856a0acc04c8be2cfc20"}

[commit-bot: I haz the power, 2017-03-03 03:54:52]

Description was changed from

==========
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
==========

to

==========
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/+/6581fa97d9ac35a9c541856a0acc...
==========

[commit-bot: I haz the power, 2017-03-03 03:54:53]

Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/6581fa97d9ac35a9c541856a0acc...

[alph, 2017-03-03 07:58:32]

A revert of this CL (patchset #8 id:140001) has been created in
https://codereview.chromium.org/2727863004/ by alph@chromium.org.

The reason for reverting is: Broke KeyRotationDeviceCloudPolicyTest.Basic test
https://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%....

[alph, 2017-03-03 21:50:40]

Description was changed from

==========
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/+/6581fa97d9ac35a9c541856a0acc...
==========

to

==========
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/+/6581fa97d9ac35a9c541856a0acc...
==========