Enable public key pinning of local trust anchors

Enable public key pinning of local trust anchors

Provides a new API that allows Cronet client to enable public key pinning of local trust anchors, i.e. pinning of certificates added to the local user trust store.

/**
 * Enables or disables pinning of the local (user-level) trust anchors.
 *
 * @param value {@code true} to enable pinning, {@code false} to disable.
 * @return the builder to facilitate chaining.
 */
 public Builder enablePublicKeyPinsForLocalTrustAnchors(boolean value);

BUG=606832
Committed: https://crrev.com/9cf8e6f923a9b472c8b3521d52b3b6ca910f77cf
Committed: https://crrev.com/385aa4234b65c226458700c8c622d705c95eab50
Cr-Original-Commit-Position: refs/heads/master@{#403486}
Cr-Commit-Position: refs/heads/master@{#403521}

[kapishnikov, 2016-06-10 17:43:50]

Description was changed from

==========
Enable public key pinning of local trust anchors

BUG=606832
==========

to

==========
Enable public key pinning of local trust anchors

Provides a new API that allows Cronet client to enable public key pinning of
local trust anchors, i.e. pinning of certificates added to the local user trust
store.

/**
 * Enables or disables pinning of the local (user-level) trust anchors.
 *
 * @param value {@code true} to enable pinning, {@code false} to disable.
 * @return the builder to facilitate chaining.
 */
 public Builder enableLocalTrustAnchorPinning(boolean value);

BUG=606832
==========

[kapishnikov, 2016-06-13 15:38:28]

kapishnikov@chromium.org changed reviewers:
+ rsleevi@chromium.org

[Ryan Sleevi, 2016-06-13 17:03:12]

https://codereview.chromium.org/2052363002/diff/1/components/cronet/android/a...
File components/cronet/android/api/src/org/chromium/net/CronetEngine.java
(right):

https://codereview.chromium.org/2052363002/diff/1/components/cronet/android/a...
components/cronet/android/api/src/org/chromium/net/CronetEngine.java:105:
private boolean mPinLocalTrustAnchors = false;
STYLE suggestion: Keep this variable named consistently with the public API

https://codereview.chromium.org/2052363002/diff/1/components/cronet/android/a...
components/cronet/android/api/src/org/chromium/net/CronetEngine.java:555:
boolean localTrustAnchorsPinningEnabled() {
naming: I'm not sufficiently read on Java style, but is it common to have
asymmetric methods like this?

Wouldn't it be
enableLocalTrustAnchorPinning
localTrustAnchorPinningEnabled

?

That still feels asymmetric w/r/t how enable/Enabled fit, perhaps

setLocalTrustAnchorPinningEnabled
localTrustAnchorPinningEnabled

Which would match the C++ style.

https://codereview.chromium.org/2052363002/diff/1/components/cronet/android/c...
File components/cronet/android/cronet_url_request_context_adapter.cc (right):

https://codereview.chromium.org/2052363002/diff/1/components/cronet/android/c...
components/cronet/android/cronet_url_request_context_adapter.cc:638:
context_->transport_security_state()->EnableLocalTrustAnchorPinning(
Naming: This C++ method name is not correct/consistent.

https://codereview.chromium.org/2052363002/diff/1/net/http/transport_security...
File net/http/transport_security_state.h (right):

https://codereview.chromium.org/2052363002/diff/1/net/http/transport_security...
net/http/transport_security_state.h:368: void EnableLocalTrustAnchorPinning(bool
value);
Naming: This does not follow the style guide
Documentation: This is insufficient documentation for the set of concerns raised
during the multiple F2F meetings.

Minimally, all Chrome consumers MUST be advised that they MUST NOT set this. See
https://www.chromium.org/Home/chromium-security/security-faq#TOC-How-does-key...

https://codereview.chromium.org/2052363002/diff/1/net/http/transport_security...
net/http/transport_security_state.h:368: void EnableLocalTrustAnchorPinning(bool
value);
DESIGN: See the discussion on https://codereview.chromium.org/2016143002/#msg35
and follows for why TransportSecurityState likely needs a few more tweaks before
introducing this. With that recent change having landed, this change would
result in inconsistent behaviour.

[kapishnikov, 2016-06-15 00:56:19]

kapishnikov@chromium.org changed reviewers:
+ mef@chromium.org, xunjieli@chromium.org

[kapishnikov, 2016-06-15 00:56:20]

Ryan, thanks for the comments. I have added the unit tests.  
I will try to merge the change with Issue 2016143002.

Adding Misha and Helen to review the Cronet part.

https://codereview.chromium.org/2052363002/diff/1/components/cronet/android/a...
File components/cronet/android/api/src/org/chromium/net/CronetEngine.java
(right):

https://codereview.chromium.org/2052363002/diff/1/components/cronet/android/a...
components/cronet/android/api/src/org/chromium/net/CronetEngine.java:105:
private boolean mPinLocalTrustAnchors = false;
On 2016/06/13 17:03:11, Ryan Sleevi wrote:
> STYLE suggestion: Keep this variable named consistently with the public API

Done, following the existing pattern.

https://codereview.chromium.org/2052363002/diff/1/components/cronet/android/a...
components/cronet/android/api/src/org/chromium/net/CronetEngine.java:555:
boolean localTrustAnchorsPinningEnabled() {
On 2016/06/13 17:03:11, Ryan Sleevi wrote:
> naming: I'm not sufficiently read on Java style, but is it common to have
> asymmetric methods like this?
> 
> Wouldn't it be
> enableLocalTrustAnchorPinning
> localTrustAnchorPinningEnabled
> 
> ?
> 
> That still feels asymmetric w/r/t how enable/Enabled fit, perhaps
> 
> setLocalTrustAnchorPinningEnabled
> localTrustAnchorPinningEnabled
> 
> Which would match the C++ style.

Point taken. I have followed the pattern how the other methods are named,
enableHTTP2/http2Enabled, enableQUIC/quicEnabled, etc. The getter methods are
not exposed to the client. I would like to keep it consistent.

https://codereview.chromium.org/2052363002/diff/1/components/cronet/android/c...
File components/cronet/android/cronet_url_request_context_adapter.cc (right):

https://codereview.chromium.org/2052363002/diff/1/components/cronet/android/c...
components/cronet/android/cronet_url_request_context_adapter.cc:638:
context_->transport_security_state()->EnableLocalTrustAnchorPinning(
On 2016/06/13 17:03:11, Ryan Sleevi wrote:
> Naming: This C++ method name is not correct/consistent.

Done.

https://codereview.chromium.org/2052363002/diff/1/net/http/transport_security...
File net/http/transport_security_state.h (right):

https://codereview.chromium.org/2052363002/diff/1/net/http/transport_security...
net/http/transport_security_state.h:368: void EnableLocalTrustAnchorPinning(bool
value);
On 2016/06/13 17:03:11, Ryan Sleevi wrote:
> Naming: This does not follow the style guide
> Documentation: This is insufficient documentation for the set of concerns
raised
> during the multiple F2F meetings.
> 
> Minimally, all Chrome consumers MUST be advised that they MUST NOT set this.
See
>
https://www.chromium.org/Home/chromium-security/security-faq#TOC-How-does-key...

Done.

https://codereview.chromium.org/2052363002/diff/1/net/http/transport_security...
net/http/transport_security_state.h:368: void EnableLocalTrustAnchorPinning(bool
value);
On 2016/06/13 17:03:12, Ryan Sleevi wrote:
> DESIGN: See the discussion on
https://codereview.chromium.org/2016143002/#msg35
> and follows for why TransportSecurityState likely needs a few more tweaks
before
> introducing this. With that recent change having landed, this change would
> result in inconsistent behaviour.

I will take a look at the change.

[xunjieli, 2016-06-20 16:54:52]

Sorry for the delay. I will take a look at the Cronet parts. The issue tracker
(crbug.com/606832) seems to suggest that there is also a design doc for this
change. Could you make the design doc public and link it in the CL description?

[kapishnikov, 2016-06-20 17:06:46]

Description was changed from

==========
Enable public key pinning of local trust anchors

Provides a new API that allows Cronet client to enable public key pinning of
local trust anchors, i.e. pinning of certificates added to the local user trust
store.

/**
 * Enables or disables pinning of the local (user-level) trust anchors.
 *
 * @param value {@code true} to enable pinning, {@code false} to disable.
 * @return the builder to facilitate chaining.
 */
 public Builder enableLocalTrustAnchorPinning(boolean value);

BUG=606832
==========

to

==========
Enable public key pinning of local trust anchors

Provides a new API that allows Cronet client to enable public key pinning of
local trust anchors, i.e. pinning of certificates added to the local user trust
store.

/**
 * Enables or disables pinning of the local (user-level) trust anchors.
 *
 * @param value {@code true} to enable pinning, {@code false} to disable.
 * @return the builder to facilitate chaining.
 */
 public Builder enablePublicKeyPinsForLocalTrustAnchors(boolean value);

BUG=606832
==========

[kapishnikov, 2016-06-20 17:11:19]

On 2016/06/20 16:54:52, xunjieli wrote:
> Sorry for the delay. I will take a look at the Cronet parts. The issue tracker
> (crbug.com/606832) seems to suggest that there is also a design doc for this
> change. Could you make the design doc public and link it in the CL
description?

The design is the Cronet API method signature that is given in the description.
The rest is the implementation. 

Also, this CL should be modified after
https://codereview.chromium.org/2066603004/ is committed; however, the
"components/cronet" part should not be affected.

[xunjieli, 2016-06-20 21:50:49]

LGTM. one suggestion below.

https://codereview.chromium.org/2052363002/diff/20001/components/cronet/andro...
File components/cronet/android/api/src/org/chromium/net/CronetEngine.java
(right):

https://codereview.chromium.org/2052363002/diff/20001/components/cronet/andro...
components/cronet/android/api/src/org/chromium/net/CronetEngine.java:550: public
Builder enablePublicKeyPinsForLocalTrustAnchors(boolean value) {
Suggest adding a brief documentation here on why we are not enabling public key
pinning for local trust anchors by default.
Maybe add a reference to this page:
https://www.chromium.org/Home/chromium-security/security-faq#TOC-How-does-key....
It isn't obvious why we are providing the option at first glance.

[xunjieli, 2016-06-20 21:54:53]

https://codereview.chromium.org/2052363002/diff/20001/components/cronet/andro...
File components/cronet/android/api/src/org/chromium/net/CronetEngine.java
(right):

https://codereview.chromium.org/2052363002/diff/20001/components/cronet/andro...
components/cronet/android/api/src/org/chromium/net/CronetEngine.java:105:
private boolean mPublicKeyPinsForLocalTrustAnchorsEnabled = false;
Suggest relying on the default initialization of boolean and remove "=false"
here to be consistent with the other booleans.

[Ryan Sleevi, 2016-06-21 00:52:26]

https://codereview.chromium.org/2052363002/diff/20001/components/cronet/andro...
File components/cronet/android/api/src/org/chromium/net/CronetEngine.java
(right):

https://codereview.chromium.org/2052363002/diff/20001/components/cronet/andro...
components/cronet/android/api/src/org/chromium/net/CronetEngine.java:545: *
Enables or disables pinning of the local (user-level) trust anchors.
This documentation does not reflect what the code does, because what the code
does is... complicated.

The way it's currently written, this suggests it allows users to pin to a custom
CA. However, that's not the goal of this feature at all, because it would also
have to be accompanied by a change to allow trusting a custom CA, and that's not
something we do.

Fundamentally, this change is the "Disrespect the user" change - in that we
ignore what the user has configured their device as, because we don't want to
(Yes, that's put pejoratively, but it's accurate, even if we did agree to do it)

More importantly, however, this *doesn't* allow user-configured trust anchors to
*override* pinning.

I don't think there's a good way to express this with an "enabled" language - or
at least, I haven't been able to find it. This
disablesPublicKeyPinningBypassForLocalTrustAnchors - but that despite the two
negatives (disable, bypass), you can't rewrite it as a positive and have it mean
the same.

IF you had to write it as a positive, because of some style requirement, then
enablePublicKeyPinningBypassForLocalTrustAnchors, which is default true. With
appropriate documentation that doing so is hostile to users and enterprises, and
should not be set (and if Cronet makes a distinction between first party/third
party consumers, more explicitly call out that first party consumers SHOULD NOT
change this)

https://codereview.chromium.org/2052363002/diff/20001/components/cronet/andro...
File components/cronet/android/test/javatests/src/org/chromium/net/PkpTest.java
(right):

https://codereview.chromium.org/2052363002/diff/20001/components/cronet/andro...
components/cronet/android/test/javatests/src/org/chromium/net/PkpTest.java:32:
private static final boolean UNKNOWN_TRUST_ROOT = false;
Drop the "TRUST" - not sure what's trying to be communicated here.

The flag is_issued_by_known_root is what's being simulated here; the 'trust'
aspect is superfluous and perhaps confusing (because it introduces another
dimension - trusted/untrusted)

https://codereview.chromium.org/2052363002/diff/20001/components/cronet/andro...
components/cronet/android/test/javatests/src/org/chromium/net/PkpTest.java:34:
private static final boolean DISABLE_LOCAL_CERT_PINNING = false;
Previous comments about naming apply here too

https://codereview.chromium.org/2052363002/diff/20001/components/cronet/andro...
File components/cronet/android/test/mock_cert_verifier.cc (right):

https://codereview.chromium.org/2052363002/diff/20001/components/cronet/andro...
components/cronet/android/test/mock_cert_verifier.cc:67: for (auto cert : certs)
{
FWIW, this should have been "const auto&", per the styleguide

https://codereview.chromium.org/2052363002/diff/20001/components/cronet/url_r...
File components/cronet/url_request_context_config.h (right):

https://codereview.chromium.org/2052363002/diff/20001/components/cronet/url_r...
components/cronet/url_request_context_config.h:109: // Enable pinning of the
locat trust anchors
See comments about documentation

https://codereview.chromium.org/2052363002/diff/20001/net/http/transport_secu...
File net/http/transport_security_state.h (right):

https://codereview.chromium.org/2052363002/diff/20001/net/http/transport_secu...
net/http/transport_security_state.h:370: //
https://www.chromium.org/Home/chromium-security/security-faq
This should get a second-pass over with the overall naming.

[kapishnikov, 2016-06-29 23:04:32]

Ryan, Helen, thanks for the review comments. I think I have addressed them all.
Also changed TransportSecurityState class to return different values of
PKPStatus enum depending on the value of
enable_pkp_bypass_for_local_trust_anchors_. I assumed that we do not want to
send reports for unknown roots (Cronet does not set REPORT_URL anyways). Same
assumption for UMA.

https://codereview.chromium.org/2052363002/diff/20001/components/cronet/andro...
File components/cronet/android/api/src/org/chromium/net/CronetEngine.java
(right):

https://codereview.chromium.org/2052363002/diff/20001/components/cronet/andro...
components/cronet/android/api/src/org/chromium/net/CronetEngine.java:105:
private boolean mPublicKeyPinsForLocalTrustAnchorsEnabled = false;
On 2016/06/20 21:54:53, xunjieli wrote:
> Suggest relying on the default initialization of boolean and remove "=false"
> here to be consistent with the other booleans.

Done.

https://codereview.chromium.org/2052363002/diff/20001/components/cronet/andro...
components/cronet/android/api/src/org/chromium/net/CronetEngine.java:545: *
Enables or disables pinning of the local (user-level) trust anchors.
On 2016/06/21 00:52:26, Ryan Sleevi wrote:
> This documentation does not reflect what the code does, because what the code
> does is... complicated.
> 
> The way it's currently written, this suggests it allows users to pin to a
custom
> CA. However, that's not the goal of this feature at all, because it would also
> have to be accompanied by a change to allow trusting a custom CA, and that's
not
> something we do.
> 
> Fundamentally, this change is the "Disrespect the user" change - in that we
> ignore what the user has configured their device as, because we don't want to
> (Yes, that's put pejoratively, but it's accurate, even if we did agree to do
it)
> 
> More importantly, however, this *doesn't* allow user-configured trust anchors
to
> *override* pinning.
> 
> I don't think there's a good way to express this with an "enabled" language -
or
> at least, I haven't been able to find it. This
> disablesPublicKeyPinningBypassForLocalTrustAnchors - but that despite the two
> negatives (disable, bypass), you can't rewrite it as a positive and have it
mean
> the same.
> 
> IF you had to write it as a positive, because of some style requirement, then
> enablePublicKeyPinningBypassForLocalTrustAnchors, which is default true. With
> appropriate documentation that doing so is hostile to users and enterprises,
and
> should not be set (and if Cronet makes a distinction between first party/third
> party consumers, more explicitly call out that first party consumers SHOULD
NOT
> change this)

We decided to go with enablePublicKeyPinningBypassForLocalTrustAnchors to be
consistent. I have also modified the JavaDoc to provide more info and discourage
the disabling of the bypass. Please feel free to suggest any modifications to
the JavaDoc.

https://codereview.chromium.org/2052363002/diff/20001/components/cronet/andro...
components/cronet/android/api/src/org/chromium/net/CronetEngine.java:550: public
Builder enablePublicKeyPinsForLocalTrustAnchors(boolean value) {
On 2016/06/20 21:50:48, xunjieli wrote:
> Suggest adding a brief documentation here on why we are not enabling public
key
> pinning for local trust anchors by default.
> Maybe add a reference to this page:
>
https://www.chromium.org/Home/chromium-security/security-faq#TOC-How-does-key....
> It isn't obvious why we are providing the option at first glance.

Done.

https://codereview.chromium.org/2052363002/diff/20001/components/cronet/andro...
File components/cronet/android/test/javatests/src/org/chromium/net/PkpTest.java
(right):

https://codereview.chromium.org/2052363002/diff/20001/components/cronet/andro...
components/cronet/android/test/javatests/src/org/chromium/net/PkpTest.java:32:
private static final boolean UNKNOWN_TRUST_ROOT = false;
On 2016/06/21 00:52:26, Ryan Sleevi wrote:
> Drop the "TRUST" - not sure what's trying to be communicated here.
> 
> The flag is_issued_by_known_root is what's being simulated here; the 'trust'
> aspect is superfluous and perhaps confusing (because it introduces another
> dimension - trusted/untrusted)

Done.

https://codereview.chromium.org/2052363002/diff/20001/components/cronet/andro...
components/cronet/android/test/javatests/src/org/chromium/net/PkpTest.java:34:
private static final boolean DISABLE_LOCAL_CERT_PINNING = false;
On 2016/06/21 00:52:26, Ryan Sleevi wrote:
> Previous comments about naming apply here too

Done.

https://codereview.chromium.org/2052363002/diff/20001/components/cronet/andro...
File components/cronet/android/test/mock_cert_verifier.cc (right):

https://codereview.chromium.org/2052363002/diff/20001/components/cronet/andro...
components/cronet/android/test/mock_cert_verifier.cc:67: for (auto cert : certs)
{
On 2016/06/21 00:52:26, Ryan Sleevi wrote:
> FWIW, this should have been "const auto&", per the styleguide

Done.

https://codereview.chromium.org/2052363002/diff/20001/components/cronet/url_r...
File components/cronet/url_request_context_config.h (right):

https://codereview.chromium.org/2052363002/diff/20001/components/cronet/url_r...
components/cronet/url_request_context_config.h:109: // Enable pinning of the
locat trust anchors
On 2016/06/21 00:52:26, Ryan Sleevi wrote:
> See comments about documentation

Renamed the variable.

https://codereview.chromium.org/2052363002/diff/20001/net/http/transport_secu...
File net/http/transport_security_state.h (right):

https://codereview.chromium.org/2052363002/diff/20001/net/http/transport_secu...
net/http/transport_security_state.h:370: //
https://www.chromium.org/Home/chromium-security/security-faq
On 2016/06/21 00:52:26, Ryan Sleevi wrote:
> This should get a second-pass over with the overall naming.

Done.

[Ryan Sleevi, 2016-07-01 01:17:24]

LGTM % nits

https://codereview.chromium.org/2052363002/diff/60001/components/cronet/andro...
File components/cronet/android/test/javatests/src/org/chromium/net/PkpTest.java
(right):

https://codereview.chromium.org/2052363002/diff/60001/components/cronet/andro...
components/cronet/android/test/javatests/src/org/chromium/net/PkpTest.java:210:
* Tests that the pinning of local trust anchors is enforced is enforced when
pinning bypass for
typo: is enforced is enforced

https://codereview.chromium.org/2052363002/diff/60001/components/cronet/url_r...
File components/cronet/url_request_context_config_unittest.cc (right):

https://codereview.chromium.org/2052363002/diff/60001/components/cronet/url_r...
components/cronet/url_request_context_config_unittest.cc:62: false, true);
Update documentation (see preceding lines)

https://codereview.chromium.org/2052363002/diff/60001/components/cronet/url_r...
components/cronet/url_request_context_config_unittest.cc:141: false, true);
Update documentation (see preceding lines)

https://codereview.chromium.org/2052363002/diff/60001/net/http/transport_secu...
File net/http/transport_security_state.cc (right):

https://codereview.chromium.org/2052363002/diff/60001/net/http/transport_secu...
net/http/transport_security_state.cc:852: }
I would argue the right thing to do, from an API consistency perspective, is

if (!is_issued_by_known_root && enable_pkp_bypass_for_local_trust_anchors_)
  return PKPStatus::BYPASSED;

That seems to fit with the API contract more

[kapishnikov, 2016-07-01 17:20:55]

https://codereview.chromium.org/2052363002/diff/60001/components/cronet/andro...
File components/cronet/android/test/javatests/src/org/chromium/net/PkpTest.java
(right):

https://codereview.chromium.org/2052363002/diff/60001/components/cronet/andro...
components/cronet/android/test/javatests/src/org/chromium/net/PkpTest.java:210:
* Tests that the pinning of local trust anchors is enforced is enforced when
pinning bypass for
On 2016/07/01 01:17:24, Ryan Sleevi wrote:
> typo: is enforced is enforced

Done.

https://codereview.chromium.org/2052363002/diff/60001/components/cronet/url_r...
File components/cronet/url_request_context_config_unittest.cc (right):

https://codereview.chromium.org/2052363002/diff/60001/components/cronet/url_r...
components/cronet/url_request_context_config_unittest.cc:62: false, true);
On 2016/07/01 01:17:24, Ryan Sleevi wrote:
> Update documentation (see preceding lines)

Done.

https://codereview.chromium.org/2052363002/diff/60001/components/cronet/url_r...
components/cronet/url_request_context_config_unittest.cc:141: false, true);
On 2016/07/01 01:17:24, Ryan Sleevi wrote:
> Update documentation (see preceding lines)

Done.

https://codereview.chromium.org/2052363002/diff/60001/net/http/transport_secu...
File net/http/transport_security_state.cc (right):

https://codereview.chromium.org/2052363002/diff/60001/net/http/transport_secu...
net/http/transport_security_state.cc:852: }
On 2016/07/01 01:17:24, Ryan Sleevi wrote:
> I would argue the right thing to do, from an API consistency perspective, is
> 
> if (!is_issued_by_known_root && enable_pkp_bypass_for_local_trust_anchors_)
>   return PKPStatus::BYPASSED;
> 
> That seems to fit with the API contract more

Agreed. Done.

[kapishnikov, 2016-07-01 17:21:01]

The CQ bit was checked by kapishnikov@chromium.org

[kapishnikov, 2016-07-01 17:21:01]

The patchset sent to the CQ was uploaded after l-g-t-m from
xunjieli@chromium.org, rsleevi@chromium.org
Link to the patchset: https://codereview.chromium.org/2052363002/#ps80001
(title: "Addressed Ryan's comments")

[commit-bot: I haz the power, 2016-07-01 17:21:19]

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-07-01 18:29:31]

Description was changed from

==========
Enable public key pinning of local trust anchors

Provides a new API that allows Cronet client to enable public key pinning of
local trust anchors, i.e. pinning of certificates added to the local user trust
store.

/**
 * Enables or disables pinning of the local (user-level) trust anchors.
 *
 * @param value {@code true} to enable pinning, {@code false} to disable.
 * @return the builder to facilitate chaining.
 */
 public Builder enablePublicKeyPinsForLocalTrustAnchors(boolean value);

BUG=606832
==========

to

==========
Enable public key pinning of local trust anchors

Provides a new API that allows Cronet client to enable public key pinning of
local trust anchors, i.e. pinning of certificates added to the local user trust
store.

/**
 * Enables or disables pinning of the local (user-level) trust anchors.
 *
 * @param value {@code true} to enable pinning, {@code false} to disable.
 * @return the builder to facilitate chaining.
 */
 public Builder enablePublicKeyPinsForLocalTrustAnchors(boolean value);

BUG=606832
==========

[commit-bot: I haz the power, 2016-07-01 18:29:32]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-07-01 18:29:49]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-01 18:32:54]

Description was changed from

==========
Enable public key pinning of local trust anchors

Provides a new API that allows Cronet client to enable public key pinning of
local trust anchors, i.e. pinning of certificates added to the local user trust
store.

/**
 * Enables or disables pinning of the local (user-level) trust anchors.
 *
 * @param value {@code true} to enable pinning, {@code false} to disable.
 * @return the builder to facilitate chaining.
 */
 public Builder enablePublicKeyPinsForLocalTrustAnchors(boolean value);

BUG=606832
==========

to

==========
Enable public key pinning of local trust anchors

Provides a new API that allows Cronet client to enable public key pinning of
local trust anchors, i.e. pinning of certificates added to the local user trust
store.

/**
 * Enables or disables pinning of the local (user-level) trust anchors.
 *
 * @param value {@code true} to enable pinning, {@code false} to disable.
 * @return the builder to facilitate chaining.
 */
 public Builder enablePublicKeyPinsForLocalTrustAnchors(boolean value);

BUG=606832

Committed: https://crrev.com/9cf8e6f923a9b472c8b3521d52b3b6ca910f77cf
Cr-Commit-Position: refs/heads/master@{#403486}
==========

[commit-bot: I haz the power, 2016-07-01 18:32:56]

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

[kelvinp, 2016-07-01 19:21:24]

A revert of this CL (patchset #5 id:80001) has been created in
https://codereview.chromium.org/2117763004/ by kelvinp@chromium.org.

The reason for reverting is: Android Bot Compilation failure:

../../components/cronet/android/test/javaperftests/src/org/chromium/net/CronetPerfTestActivity.java:196:
error: method createMockCertVerifier in class MockCertVerifier cannot be applied
to given types;

https://build.chromium.org/p/chromium/builders/Android/builds/58499/steps/com...

.

[kapishnikov, 2016-07-01 20:08:49]

Description was changed from

==========
Enable public key pinning of local trust anchors

Provides a new API that allows Cronet client to enable public key pinning of
local trust anchors, i.e. pinning of certificates added to the local user trust
store.

/**
 * Enables or disables pinning of the local (user-level) trust anchors.
 *
 * @param value {@code true} to enable pinning, {@code false} to disable.
 * @return the builder to facilitate chaining.
 */
 public Builder enablePublicKeyPinsForLocalTrustAnchors(boolean value);

BUG=606832

Committed: https://crrev.com/9cf8e6f923a9b472c8b3521d52b3b6ca910f77cf
Cr-Commit-Position: refs/heads/master@{#403486}
==========

to

==========
Enable public key pinning of local trust anchors

Provides a new API that allows Cronet client to enable public key pinning of
local trust anchors, i.e. pinning of certificates added to the local user trust
store.

/**
 * Enables or disables pinning of the local (user-level) trust anchors.
 *
 * @param value {@code true} to enable pinning, {@code false} to disable.
 * @return the builder to facilitate chaining.
 */
 public Builder enablePublicKeyPinsForLocalTrustAnchors(boolean value);

BUG=606832

Committed: https://crrev.com/9cf8e6f923a9b472c8b3521d52b3b6ca910f77cf
Cr-Commit-Position: refs/heads/master@{#403486}
==========

[kapishnikov, 2016-07-01 20:30:42]

The CQ bit was checked by kapishnikov@chromium.org

[kapishnikov, 2016-07-01 20:30:42]

The patchset sent to the CQ was uploaded after l-g-t-m from
rsleevi@chromium.org, xunjieli@chromium.org
Link to the patchset: https://codereview.chromium.org/2052363002/#ps100001
(title: "Fixed CronetPerfTestActivity test")

[commit-bot: I haz the power, 2016-07-01 20:32:00]

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-07-01 20:53:14]

Description was changed from

==========
Enable public key pinning of local trust anchors

Provides a new API that allows Cronet client to enable public key pinning of
local trust anchors, i.e. pinning of certificates added to the local user trust
store.

/**
 * Enables or disables pinning of the local (user-level) trust anchors.
 *
 * @param value {@code true} to enable pinning, {@code false} to disable.
 * @return the builder to facilitate chaining.
 */
 public Builder enablePublicKeyPinsForLocalTrustAnchors(boolean value);

BUG=606832

Committed: https://crrev.com/9cf8e6f923a9b472c8b3521d52b3b6ca910f77cf
Cr-Commit-Position: refs/heads/master@{#403486}
==========

to

==========
Enable public key pinning of local trust anchors

Provides a new API that allows Cronet client to enable public key pinning of
local trust anchors, i.e. pinning of certificates added to the local user trust
store.

/**
 * Enables or disables pinning of the local (user-level) trust anchors.
 *
 * @param value {@code true} to enable pinning, {@code false} to disable.
 * @return the builder to facilitate chaining.
 */
 public Builder enablePublicKeyPinsForLocalTrustAnchors(boolean value);

BUG=606832

Committed: https://crrev.com/9cf8e6f923a9b472c8b3521d52b3b6ca910f77cf
Cr-Commit-Position: refs/heads/master@{#403486}
==========

[commit-bot: I haz the power, 2016-07-01 20:53:16]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-07-01 20:53:19]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-01 20:54:44]

Description was changed from

==========
Enable public key pinning of local trust anchors

Provides a new API that allows Cronet client to enable public key pinning of
local trust anchors, i.e. pinning of certificates added to the local user trust
store.

/**
 * Enables or disables pinning of the local (user-level) trust anchors.
 *
 * @param value {@code true} to enable pinning, {@code false} to disable.
 * @return the builder to facilitate chaining.
 */
 public Builder enablePublicKeyPinsForLocalTrustAnchors(boolean value);

BUG=606832

Committed: https://crrev.com/9cf8e6f923a9b472c8b3521d52b3b6ca910f77cf
Cr-Commit-Position: refs/heads/master@{#403486}
==========

to

==========
Enable public key pinning of local trust anchors

Provides a new API that allows Cronet client to enable public key pinning of
local trust anchors, i.e. pinning of certificates added to the local user trust
store.

/**
 * Enables or disables pinning of the local (user-level) trust anchors.
 *
 * @param value {@code true} to enable pinning, {@code false} to disable.
 * @return the builder to facilitate chaining.
 */
 public Builder enablePublicKeyPinsForLocalTrustAnchors(boolean value);

BUG=606832

Committed: https://crrev.com/9cf8e6f923a9b472c8b3521d52b3b6ca910f77cf
Committed: https://crrev.com/385aa4234b65c226458700c8c622d705c95eab50
Cr-Original-Commit-Position: refs/heads/master@{#403486}
Cr-Commit-Position: refs/heads/master@{#403521}
==========

[commit-bot: I haz the power, 2016-07-01 20:54:45]

Patchset 6 (id:??) landed as
https://crrev.com/385aa4234b65c226458700c8c622d705c95eab50
Cr-Commit-Position: refs/heads/master@{#403521}