New issue
Advanced search Search tips

Issue 779166 link

Starred by 19 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 24
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac , Fuchsia
Pri: 1
Type: Feature

Blocked on:
issue 890409



Sign in to add a comment

Deprecate And Remove header-based HTTP Public Key Pinning (HPKP)

Project Member Reported by palmer@chromium.org, Oct 27 2017

Issue description

This is a tracking bug for work on deprecating and removing key pinning. Use it for related bugs and CLs, such as any developer console warnings, additional metrics, removing the support code, et c.
 
Cc: lgar...@chromium.org
FWIW, I'm still accepting new preloaded pinning requests for the time being (one every few months) until we have a concrete decision otherwise. This bug looks like a good place for such a decision, in case anyone feels we should stop already.

Comment 2 by palmer@chromium.org, Oct 27 2017

#1: I think you should probably stop accepting them.
See also Issue 404461.
Labels: -Pri-2 -M-67 M-69 Pri-1
OK, we're looking at removing dynamic PKP in M69. Static PKP will remain until further notice (we have no active plans to remove it right now).
Cc: asymmetric@chromium.org
Cc: emilyschechter@chromium.org
+emilyschechter: Do you think we need an un-launch bug for this?
Project Member

Comment 7 by bugdroid1@chromium.org, Apr 12 2018

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

commit a136eabd44f63725d3e40560a0b29c3115334fbb
Author: Chris Palmer <palmer@chromium.org>
Date: Thu Apr 12 23:50:26 2018

Log a deprecation warning for HTTP-Based Public Key Pinning.

When we see the header, print a warning to the console.

Adapted from a suggestion from elawrence@.

Bug:  779166 
Change-Id: Id68495a37f43436cd51833391dbf140d5eb9aef3
Reviewed-on: https://chromium-review.googlesource.com/1006194
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: Eric Lawrence <elawrence@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550417}
[modify] https://crrev.com/a136eabd44f63725d3e40560a0b29c3115334fbb/third_party/blink/renderer/devtools/front_end/sdk/NetworkManager.js

Project Member

Comment 8 by bugdroid1@chromium.org, Apr 17 2018

Labels: merge-merged-testbranch
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a136eabd44f63725d3e40560a0b29c3115334fbb

commit a136eabd44f63725d3e40560a0b29c3115334fbb
Author: Chris Palmer <palmer@chromium.org>
Date: Thu Apr 12 23:50:26 2018

Log a deprecation warning for HTTP-Based Public Key Pinning.

When we see the header, print a warning to the console.

Adapted from a suggestion from elawrence@.

Bug:  779166 
Change-Id: Id68495a37f43436cd51833391dbf140d5eb9aef3
Reviewed-on: https://chromium-review.googlesource.com/1006194
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: Eric Lawrence <elawrence@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550417}
[modify] https://crrev.com/a136eabd44f63725d3e40560a0b29c3115334fbb/third_party/blink/renderer/devtools/front_end/sdk/NetworkManager.js

Project Member

Comment 9 by bugdroid1@chromium.org, Jun 1 2018

Project Member

Comment 10 by bugdroid1@chromium.org, Jun 7 2018

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

commit 45a059dc425e08165f9a10324bd1380cc13ca363
Author: Chris Palmer <palmer@chromium.org>
Date: Thu Jun 07 20:14:56 2018

[HPKP] Use static pins for testing static pin reporting.

Since dynamic pins are going away.

Bug:  779166 
Change-Id: I3872a872b84f0780facd161f7abb88990a872b31
Reviewed-on: https://chromium-review.googlesource.com/1087646
Commit-Queue: Chris Palmer <palmer@chromium.org>
Reviewed-by: Emily Stark <estark@chromium.org>
Reviewed-by: Nick Harper <nharper@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565386}
[modify] https://crrev.com/45a059dc425e08165f9a10324bd1380cc13ca363/net/http/transport_security_state_static.json
[modify] https://crrev.com/45a059dc425e08165f9a10324bd1380cc13ca363/net/http/transport_security_state_unittest.cc

Cc: -lgar...@chromium.org -rsleevi@chromium.org palmer@chromium.org
Owner: rsleevi@chromium.org
Passing to rsleevi to take or to reassign, as appropriate.

CLs in progress:

Full size: https://chromium-review.googlesource.com/c/chromium/src/+/1005960/

Smaller version with fewer tests refactored: https://chromium-review.googlesource.com/c/chromium/src/+/1199493
Cc: rsleevi@chromium.org
Owner: mmenke@chromium.org
Redirecting to Matt to see if he can find a better owner.

The remaining steps were blocked on ensuring functional integration tests in the presence of the Network Service, as the current browser_tests rely on using Dynamic HPKP to test behaviours such as sending reports. This potentially involves adding test-only interfaces (NetworkServiceTest?) to configure which static source is used (and to enable the static data, which is default-disabled for Chromium builds), so that it can use test-only data to make sure the features continue to work.
I'm confused.  We need tests for a feature before we remove that feature?  Is this bug only for deprecating the dynamic list, and not the static one, and you want tests for the static lists?
@mmenke:

Correct. The removal first involves removing dynamic HPKP. Currently, the tests for HPKP reporting (usable with both static and dynamic PKP) use the TestServer w/ dynamic PKP in order to test that reports are sent.

By removing dynamic PKP support, there's no way to test the PKP reporting from the browser_tests side dynamically. The way this is being done w/in the NetworkService & friends is to use some test-only methods on the TransportSecurityState (TSS::EnableStaticPinsForTesting, SetTransportSecurityStateSourceForTesting) to set up a test-only 'static' configuration that configures both domains to report from (when accessed) and the domains to report to.

Palmer's first CL ("full size") had everything working except for those browser_test migrations.
There are currently tests to check that behaviors like sending reports (a feature of PKP that exists for both dynamic and static entries) work correctly. These tests are currently implemented using the dynamic code, and it's nontrivial to reimplement them using only a test static PKP list. (I helped Palmer rewrite some of them, but browser_tests is the largest remaining one to get working with the static list instead of dynamic entries.)
Think you're going to need an cert expert figure out how to implement this, cert-side, before anything can be hooked up to a test API.  I assume we'd want to inject cert results at a lower layer than the cert verifier for the built-in certs.
No, that is not necessary. The existing testing interfaces for MockCertVerifier are sufficient.

https://chromium-review.googlesource.com/c/chromium/src/+/1005960/35/chrome/browser/ssl/ssl_browsertest.cc#7697 captures more discussion.
Then what's missing?  Is this just a port issue, with the built-in values pointing reports to the wrong port (Since we can intercept DNS lookups, and mock cert verification results, but not mock the port used for the requests), or is the issue that they are only enabled in cert configurations?  (Or both)
Oops, sorry, missed the link.
I *think* the port issue (the report_uri entry in the unittest default pinset is for the default port, port 80, whereas the embedded_test_server needs to receive the request on the test server's ephemeral port) is the main thing that was blocking me getting the browser test working. But in addition to that, it sounds like there's something that would need to get wired thru the network service as well to make that test work when the network service is enabled (I'm fuzzy on what exactly that bit is).
So the options are to either inject a "static" HSTS entry (Do we have code for that?) or make things work with a real entry (Which would require either intercepting the report at the net/ layer, or I guess with a proxy - the proxy solution might be handy, as it could be reused in similar situations, and probably wouldn't be too much code, but does seem a little hairy.  We'd need to have minimal bogus proxy (SOCKS5 / HTTP) handshake code above the SSL layer for the embedded test server).  The most sane way is presumably injecting the static entry.

Either way, we'd need to enable the static HSTS list for a non-Chrome build as well, and given concerns over it breaking third parties, we want a test-only interface there, instead of just enabling it for content-shell.

This doesn't really seem to depend much on knowledge of the NetworkService, but more knowledge of how to muck with TransportSecurityState (Assuming we go with static HSTS injection - if we go with the other options, HttpJob interception or embedded test server faking a proxy, things are a bit different)...Or am I missing something?
> So the options are to either inject a "static" HSTS entry (Do we have code for that?)

Yes. Noted in the CL in Comment #17 and mentioned in Comment #14.

> we want a test-only interface there

Yes. //net has that (mentioned in Comment #14) and is what the existing lower layer tests are using - but that code runs in the Network Service.


Basically, we have the same tests running at different layers - //net, the Network Service, and browser_tests - functionally acting as unit tests, feature tests, and integration tests. For the former two, it's easier to make sure things 'work', because they run in process and the existing test interfaces work. For the latter - browser_tests - there's at minimum two methods to expose on the Network Service (enable static pins, set the pin source), and possibly a third (which is setup a URL interceptor or the like to respond to the request on the 'wrong' port with a redirect to the 'right' port, or a proxy, or simply dynamically compute the right pin source).

One of the stopping points was NetworkServiceTest not working on MacOS, thus presenting some challenges, but that's now resolved. The rest is just someone to drive the (presumably, NetworkServiceTest) changes through in a way they're OK with.
Cc: mmenke@chromium.org
Owner: mattm@chromium.org
Status: Started (was: Assigned)
I'll have a go at getting this finished off.
Cc: mef@chromium.org
While going over the change I've noticed that Cronet allows you to setup PKP pins in its URLRequestContextConfig, which utilizes the dynamic HPKP implementation, so that will also be disabled by the deprecation of dynamic HPKP. Is that already known/expected?

https://cs.chromium.org/chromium/src/components/cronet/url_request_context_config.h?rcl=611ccce4d6e5daba483076755778d8ef77220693&l=63
mattm@: This is not expected, thanks for pointing this out! 
Is entire PKP support getting deprecated or just dynamic PKP?

We have a customer that uses PKP on Android and iOS, so we need to figure out a good path forward.
We intend only to deprecate dynamic PKP for now.
Blocking: 890409
Although Cronet uses the dynamic PKP code, it doesn't use any of the storage bits or whatever. It just wants to register them in memory. So that would still allow the bulk of the code to go. And then once the interfaces are only for Cronet's use case, we can probably simplify that.
Blocking: -890409
Blockedon: 890409
Project Member

Comment 31 by bugdroid1@chromium.org, Oct 11

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

commit e211b725cdb2b5e0e7cb37f45f2126eb09780562
Author: Matt Mueller <mattm@chromium.org>
Date: Thu Oct 11 03:38:10 2018

Remove HTTP-Based Public Key Pinning header parsing and persistence code.

And related code that uses it.

Cronet depends on the base dynamic PKP support, so is not removed here.

Based on https://crrev.com/c/1005960 by palmer & nharper.

Bug:  779166 
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I44044a3960174fcba1f1e120b18cbef3ff769812
Reviewed-on: https://chromium-review.googlesource.com/c/1260483
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Ryan Sleevi <rsleevi@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: Ryan Sleevi <rsleevi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598657}
[modify] https://crrev.com/e211b725cdb2b5e0e7cb37f45f2126eb09780562/chrome/browser/resources/net_internals/domain_security_policy_view.js
[modify] https://crrev.com/e211b725cdb2b5e0e7cb37f45f2126eb09780562/chrome/browser/ssl/security_state_tab_helper_browsertest.cc
[modify] https://crrev.com/e211b725cdb2b5e0e7cb37f45f2126eb09780562/chrome/browser/ssl/ssl_browsertest.cc
[modify] https://crrev.com/e211b725cdb2b5e0e7cb37f45f2126eb09780562/chrome/browser/ui/webui/net_internals/net_internals_ui.cc
[modify] https://crrev.com/e211b725cdb2b5e0e7cb37f45f2126eb09780562/content/public/test/network_service_test_helper.cc
[modify] https://crrev.com/e211b725cdb2b5e0e7cb37f45f2126eb09780562/net/BUILD.gn
[modify] https://crrev.com/e211b725cdb2b5e0e7cb37f45f2126eb09780562/net/data/ssl/certificates/spdy_pooling.pem
[modify] https://crrev.com/e211b725cdb2b5e0e7cb37f45f2126eb09780562/net/data/ssl/scripts/ee.cnf
[delete] https://crrev.com/8a58a983c4e83c6a67026962eae3b097a298c002/net/data/url_request_unittest/hpkp-headers-report-only.html
[delete] https://crrev.com/8a58a983c4e83c6a67026962eae3b097a298c002/net/data/url_request_unittest/hpkp-headers-report-only.html.mock-http-headers
[delete] https://crrev.com/8a58a983c4e83c6a67026962eae3b097a298c002/net/data/url_request_unittest/hpkp-headers.html
[delete] https://crrev.com/8a58a983c4e83c6a67026962eae3b097a298c002/net/data/url_request_unittest/hpkp-headers.html.mock-http-headers
[delete] https://crrev.com/8a58a983c4e83c6a67026962eae3b097a298c002/net/data/url_request_unittest/hsts-and-hpkp-headers.html
[delete] https://crrev.com/8a58a983c4e83c6a67026962eae3b097a298c002/net/data/url_request_unittest/hsts-and-hpkp-headers.html.mock-http-headers
[delete] https://crrev.com/8a58a983c4e83c6a67026962eae3b097a298c002/net/data/url_request_unittest/hsts-and-hpkp-headers2.html
[delete] https://crrev.com/8a58a983c4e83c6a67026962eae3b097a298c002/net/data/url_request_unittest/hsts-and-hpkp-headers2.html.mock-http-headers
[modify] https://crrev.com/e211b725cdb2b5e0e7cb37f45f2126eb09780562/net/http/http_response_headers.cc
[modify] https://crrev.com/e211b725cdb2b5e0e7cb37f45f2126eb09780562/net/http/http_security_headers.cc
[modify] https://crrev.com/e211b725cdb2b5e0e7cb37f45f2126eb09780562/net/http/http_security_headers.h
[delete] https://crrev.com/8a58a983c4e83c6a67026962eae3b097a298c002/net/http/http_security_headers_hpkp_fuzzer.cc
[delete] https://crrev.com/8a58a983c4e83c6a67026962eae3b097a298c002/net/http/http_security_headers_hpkp_report_only_fuzzer.cc
[modify] https://crrev.com/e211b725cdb2b5e0e7cb37f45f2126eb09780562/net/http/http_security_headers_unittest.cc
[modify] https://crrev.com/e211b725cdb2b5e0e7cb37f45f2126eb09780562/net/http/transport_security_persister.cc
[modify] https://crrev.com/e211b725cdb2b5e0e7cb37f45f2126eb09780562/net/http/transport_security_persister.h
[modify] https://crrev.com/e211b725cdb2b5e0e7cb37f45f2126eb09780562/net/http/transport_security_persister_unittest.cc
[modify] https://crrev.com/e211b725cdb2b5e0e7cb37f45f2126eb09780562/net/http/transport_security_state.cc
[modify] https://crrev.com/e211b725cdb2b5e0e7cb37f45f2126eb09780562/net/http/transport_security_state.h
[modify] https://crrev.com/e211b725cdb2b5e0e7cb37f45f2126eb09780562/net/http/transport_security_state_static_unittest_default.json
[add] https://crrev.com/e211b725cdb2b5e0e7cb37f45f2126eb09780562/net/http/transport_security_state_test_util.cc
[add] https://crrev.com/e211b725cdb2b5e0e7cb37f45f2126eb09780562/net/http/transport_security_state_test_util.h
[modify] https://crrev.com/e211b725cdb2b5e0e7cb37f45f2126eb09780562/net/http/transport_security_state_unittest.cc
[modify] https://crrev.com/e211b725cdb2b5e0e7cb37f45f2126eb09780562/net/quic/crypto/proof_verifier_chromium_test.cc
[modify] https://crrev.com/e211b725cdb2b5e0e7cb37f45f2126eb09780562/net/quic/quic_chromium_client_session_test.cc
[modify] https://crrev.com/e211b725cdb2b5e0e7cb37f45f2126eb09780562/net/quic/quic_stream_factory_test.cc
[modify] https://crrev.com/e211b725cdb2b5e0e7cb37f45f2126eb09780562/net/socket/ssl_client_socket_unittest.cc
[modify] https://crrev.com/e211b725cdb2b5e0e7cb37f45f2126eb09780562/net/spdy/spdy_session_unittest.cc
[modify] https://crrev.com/e211b725cdb2b5e0e7cb37f45f2126eb09780562/net/spdy/spdy_test_util_common.cc
[modify] https://crrev.com/e211b725cdb2b5e0e7cb37f45f2126eb09780562/net/spdy/spdy_test_util_common.h
[modify] https://crrev.com/e211b725cdb2b5e0e7cb37f45f2126eb09780562/net/test/embedded_test_server/embedded_test_server.cc
[modify] https://crrev.com/e211b725cdb2b5e0e7cb37f45f2126eb09780562/net/test/embedded_test_server/embedded_test_server.h
[modify] https://crrev.com/e211b725cdb2b5e0e7cb37f45f2126eb09780562/net/url_request/url_request_http_job.cc
[modify] https://crrev.com/e211b725cdb2b5e0e7cb37f45f2126eb09780562/net/url_request/url_request_http_job.h
[modify] https://crrev.com/e211b725cdb2b5e0e7cb37f45f2126eb09780562/net/url_request/url_request_unittest.cc
[modify] https://crrev.com/e211b725cdb2b5e0e7cb37f45f2126eb09780562/services/network/BUILD.gn
[modify] https://crrev.com/e211b725cdb2b5e0e7cb37f45f2126eb09780562/services/network/network_context.cc
[modify] https://crrev.com/e211b725cdb2b5e0e7cb37f45f2126eb09780562/services/network/network_context.h
[modify] https://crrev.com/e211b725cdb2b5e0e7cb37f45f2126eb09780562/services/network/network_context_unittest.cc
[modify] https://crrev.com/e211b725cdb2b5e0e7cb37f45f2126eb09780562/services/network/public/mojom/network_context.mojom
[modify] https://crrev.com/e211b725cdb2b5e0e7cb37f45f2126eb09780562/services/network/public/mojom/network_service_test.mojom
[modify] https://crrev.com/e211b725cdb2b5e0e7cb37f45f2126eb09780562/services/network/test/test_network_context.h

Project Member

Comment 32 by bugdroid1@chromium.org, Oct 11

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

commit 4e4967bcb6c94edcdb8c190686452d6383c086f4
Author: Hiroki Nakagawa <nhiroki@chromium.org>
Date: Thu Oct 11 04:09:51 2018

Revert "Remove HTTP-Based Public Key Pinning header parsing and persistence code."

This reverts commit e211b725cdb2b5e0e7cb37f45f2126eb09780562.

Reason for revert:
This is failing NetworkContextTest.CertReporting test in service_unittests:
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/linux-ozone-rel/36048
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Cast%20Audio%20Linux/22797

Original change's description:
> Remove HTTP-Based Public Key Pinning header parsing and persistence code.
> 
> And related code that uses it.
> 
> Cronet depends on the base dynamic PKP support, so is not removed here.
> 
> Based on https://crrev.com/c/1005960 by palmer & nharper.
> 
> Bug:  779166 
> Cq-Include-Trybots: luci.chromium.try:linux_mojo
> Change-Id: I44044a3960174fcba1f1e120b18cbef3ff769812
> Reviewed-on: https://chromium-review.googlesource.com/c/1260483
> Reviewed-by: Tom Sepez <tsepez@chromium.org>
> Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> Reviewed-by: Ryan Sleevi <rsleevi@chromium.org>
> Reviewed-by: Matt Menke <mmenke@chromium.org>
> Commit-Queue: Ryan Sleevi <rsleevi@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#598657}

TBR=rsleevi@chromium.org,jam@chromium.org,mattm@chromium.org,mmenke@chromium.org,tsepez@chromium.org

Change-Id: Id7ee1c2284e1cd95ac48a92bfad3dfae58380822
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  779166 
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Reviewed-on: https://chromium-review.googlesource.com/c/1275507
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598666}
[modify] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/chrome/browser/resources/net_internals/domain_security_policy_view.js
[modify] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/chrome/browser/ssl/security_state_tab_helper_browsertest.cc
[modify] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/chrome/browser/ssl/ssl_browsertest.cc
[modify] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/chrome/browser/ui/webui/net_internals/net_internals_ui.cc
[modify] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/content/public/test/network_service_test_helper.cc
[modify] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/net/BUILD.gn
[modify] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/net/data/ssl/certificates/spdy_pooling.pem
[modify] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/net/data/ssl/scripts/ee.cnf
[add] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/net/data/url_request_unittest/hpkp-headers-report-only.html
[add] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/net/data/url_request_unittest/hpkp-headers-report-only.html.mock-http-headers
[add] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/net/data/url_request_unittest/hpkp-headers.html
[add] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/net/data/url_request_unittest/hpkp-headers.html.mock-http-headers
[add] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/net/data/url_request_unittest/hsts-and-hpkp-headers.html
[add] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/net/data/url_request_unittest/hsts-and-hpkp-headers.html.mock-http-headers
[add] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/net/data/url_request_unittest/hsts-and-hpkp-headers2.html
[add] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/net/data/url_request_unittest/hsts-and-hpkp-headers2.html.mock-http-headers
[modify] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/net/http/http_response_headers.cc
[modify] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/net/http/http_security_headers.cc
[modify] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/net/http/http_security_headers.h
[add] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/net/http/http_security_headers_hpkp_fuzzer.cc
[add] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/net/http/http_security_headers_hpkp_report_only_fuzzer.cc
[modify] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/net/http/http_security_headers_unittest.cc
[modify] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/net/http/transport_security_persister.cc
[modify] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/net/http/transport_security_persister.h
[modify] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/net/http/transport_security_persister_unittest.cc
[modify] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/net/http/transport_security_state.cc
[modify] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/net/http/transport_security_state.h
[modify] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/net/http/transport_security_state_static_unittest_default.json
[delete] https://crrev.com/9c2d9c027d9ea879fdecf4dbd5f09aee7437e264/net/http/transport_security_state_test_util.cc
[delete] https://crrev.com/9c2d9c027d9ea879fdecf4dbd5f09aee7437e264/net/http/transport_security_state_test_util.h
[modify] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/net/http/transport_security_state_unittest.cc
[modify] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/net/quic/crypto/proof_verifier_chromium_test.cc
[modify] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/net/quic/quic_chromium_client_session_test.cc
[modify] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/net/quic/quic_stream_factory_test.cc
[modify] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/net/socket/ssl_client_socket_unittest.cc
[modify] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/net/spdy/spdy_session_unittest.cc
[modify] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/net/spdy/spdy_test_util_common.cc
[modify] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/net/spdy/spdy_test_util_common.h
[modify] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/net/test/embedded_test_server/embedded_test_server.cc
[modify] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/net/test/embedded_test_server/embedded_test_server.h
[modify] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/net/url_request/url_request_http_job.cc
[modify] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/net/url_request/url_request_http_job.h
[modify] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/net/url_request/url_request_unittest.cc
[modify] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/services/network/BUILD.gn
[modify] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/services/network/network_context.cc
[modify] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/services/network/network_context.h
[modify] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/services/network/network_context_unittest.cc
[modify] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/services/network/public/mojom/network_context.mojom
[modify] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/services/network/public/mojom/network_service_test.mojom
[modify] https://crrev.com/4e4967bcb6c94edcdb8c190686452d6383c086f4/services/network/test/test_network_context.h

Project Member

Comment 33 by bugdroid1@chromium.org, Oct 22

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

commit 230996f183bc6eb2f8ec2a39f7d19f9d6267ec84
Author: Matt Mueller <mattm@chromium.org>
Date: Mon Oct 22 19:39:44 2018

Reland "Remove HTTP-Based Public Key Pinning header parsing and persistence code."

This is a reland of e211b725cdb2b5e0e7cb37f45f2126eb09780562,
with fix for NetworkContextTest.CertReporting (restored original behavior of the
test using a testserver to listen for the pkp report).

Original change's description:
> Remove HTTP-Based Public Key Pinning header parsing and persistence code.
>
> And related code that uses it.
>
> Cronet depends on the base dynamic PKP support, so is not removed here.
>
> Based on https://crrev.com/c/1005960 by palmer & nharper.
>
> Bug:  779166 
> Cq-Include-Trybots: luci.chromium.try:linux_mojo
> Change-Id: I44044a3960174fcba1f1e120b18cbef3ff769812
> Reviewed-on: https://chromium-review.googlesource.com/c/1260483
> Reviewed-by: Tom Sepez <tsepez@chromium.org>
> Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> Reviewed-by: Ryan Sleevi <rsleevi@chromium.org>
> Reviewed-by: Matt Menke <mmenke@chromium.org>
> Commit-Queue: Ryan Sleevi <rsleevi@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#598657}

TBR=jam@chromium.org

Bug:  779166 
Change-Id: I2e3b4dbab9cbbc95606c1cf7051b112fc5ba8cc4
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Reviewed-on: https://chromium-review.googlesource.com/c/1277963
Commit-Queue: Matt Mueller <mattm@chromium.org>
Reviewed-by: Ryan Sleevi <rsleevi@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601687}
[modify] https://crrev.com/230996f183bc6eb2f8ec2a39f7d19f9d6267ec84/chrome/browser/resources/net_internals/domain_security_policy_view.js
[modify] https://crrev.com/230996f183bc6eb2f8ec2a39f7d19f9d6267ec84/chrome/browser/ssl/security_state_tab_helper_browsertest.cc
[modify] https://crrev.com/230996f183bc6eb2f8ec2a39f7d19f9d6267ec84/chrome/browser/ssl/ssl_browsertest.cc
[modify] https://crrev.com/230996f183bc6eb2f8ec2a39f7d19f9d6267ec84/chrome/browser/ui/webui/net_internals/net_internals_ui.cc
[modify] https://crrev.com/230996f183bc6eb2f8ec2a39f7d19f9d6267ec84/content/public/test/network_service_test_helper.cc
[modify] https://crrev.com/230996f183bc6eb2f8ec2a39f7d19f9d6267ec84/net/BUILD.gn
[modify] https://crrev.com/230996f183bc6eb2f8ec2a39f7d19f9d6267ec84/net/data/ssl/certificates/spdy_pooling.pem
[modify] https://crrev.com/230996f183bc6eb2f8ec2a39f7d19f9d6267ec84/net/data/ssl/scripts/ee.cnf
[delete] https://crrev.com/9385a4e282fe3ddb35a067dd68f0b1e3164e7070/net/data/url_request_unittest/hpkp-headers-report-only.html
[delete] https://crrev.com/9385a4e282fe3ddb35a067dd68f0b1e3164e7070/net/data/url_request_unittest/hpkp-headers-report-only.html.mock-http-headers
[delete] https://crrev.com/9385a4e282fe3ddb35a067dd68f0b1e3164e7070/net/data/url_request_unittest/hpkp-headers.html
[delete] https://crrev.com/9385a4e282fe3ddb35a067dd68f0b1e3164e7070/net/data/url_request_unittest/hpkp-headers.html.mock-http-headers
[delete] https://crrev.com/9385a4e282fe3ddb35a067dd68f0b1e3164e7070/net/data/url_request_unittest/hsts-and-hpkp-headers.html
[delete] https://crrev.com/9385a4e282fe3ddb35a067dd68f0b1e3164e7070/net/data/url_request_unittest/hsts-and-hpkp-headers.html.mock-http-headers
[delete] https://crrev.com/9385a4e282fe3ddb35a067dd68f0b1e3164e7070/net/data/url_request_unittest/hsts-and-hpkp-headers2.html
[delete] https://crrev.com/9385a4e282fe3ddb35a067dd68f0b1e3164e7070/net/data/url_request_unittest/hsts-and-hpkp-headers2.html.mock-http-headers
[modify] https://crrev.com/230996f183bc6eb2f8ec2a39f7d19f9d6267ec84/net/http/http_response_headers.cc
[modify] https://crrev.com/230996f183bc6eb2f8ec2a39f7d19f9d6267ec84/net/http/http_security_headers.cc
[modify] https://crrev.com/230996f183bc6eb2f8ec2a39f7d19f9d6267ec84/net/http/http_security_headers.h
[delete] https://crrev.com/9385a4e282fe3ddb35a067dd68f0b1e3164e7070/net/http/http_security_headers_hpkp_fuzzer.cc
[delete] https://crrev.com/9385a4e282fe3ddb35a067dd68f0b1e3164e7070/net/http/http_security_headers_hpkp_report_only_fuzzer.cc
[modify] https://crrev.com/230996f183bc6eb2f8ec2a39f7d19f9d6267ec84/net/http/http_security_headers_unittest.cc
[modify] https://crrev.com/230996f183bc6eb2f8ec2a39f7d19f9d6267ec84/net/http/transport_security_persister.cc
[modify] https://crrev.com/230996f183bc6eb2f8ec2a39f7d19f9d6267ec84/net/http/transport_security_persister.h
[modify] https://crrev.com/230996f183bc6eb2f8ec2a39f7d19f9d6267ec84/net/http/transport_security_persister_unittest.cc
[modify] https://crrev.com/230996f183bc6eb2f8ec2a39f7d19f9d6267ec84/net/http/transport_security_state.cc
[modify] https://crrev.com/230996f183bc6eb2f8ec2a39f7d19f9d6267ec84/net/http/transport_security_state.h
[modify] https://crrev.com/230996f183bc6eb2f8ec2a39f7d19f9d6267ec84/net/http/transport_security_state_static_unittest_default.json
[add] https://crrev.com/230996f183bc6eb2f8ec2a39f7d19f9d6267ec84/net/http/transport_security_state_test_util.cc
[add] https://crrev.com/230996f183bc6eb2f8ec2a39f7d19f9d6267ec84/net/http/transport_security_state_test_util.h
[modify] https://crrev.com/230996f183bc6eb2f8ec2a39f7d19f9d6267ec84/net/http/transport_security_state_unittest.cc
[modify] https://crrev.com/230996f183bc6eb2f8ec2a39f7d19f9d6267ec84/net/quic/crypto/proof_verifier_chromium_test.cc
[modify] https://crrev.com/230996f183bc6eb2f8ec2a39f7d19f9d6267ec84/net/quic/quic_chromium_client_session_test.cc
[modify] https://crrev.com/230996f183bc6eb2f8ec2a39f7d19f9d6267ec84/net/quic/quic_stream_factory_test.cc
[modify] https://crrev.com/230996f183bc6eb2f8ec2a39f7d19f9d6267ec84/net/socket/ssl_client_socket_unittest.cc
[modify] https://crrev.com/230996f183bc6eb2f8ec2a39f7d19f9d6267ec84/net/spdy/spdy_session_unittest.cc
[modify] https://crrev.com/230996f183bc6eb2f8ec2a39f7d19f9d6267ec84/net/spdy/spdy_test_util_common.cc
[modify] https://crrev.com/230996f183bc6eb2f8ec2a39f7d19f9d6267ec84/net/spdy/spdy_test_util_common.h
[modify] https://crrev.com/230996f183bc6eb2f8ec2a39f7d19f9d6267ec84/net/test/embedded_test_server/embedded_test_server.cc
[modify] https://crrev.com/230996f183bc6eb2f8ec2a39f7d19f9d6267ec84/net/test/embedded_test_server/embedded_test_server.h
[modify] https://crrev.com/230996f183bc6eb2f8ec2a39f7d19f9d6267ec84/net/url_request/url_request_http_job.cc
[modify] https://crrev.com/230996f183bc6eb2f8ec2a39f7d19f9d6267ec84/net/url_request/url_request_http_job.h
[modify] https://crrev.com/230996f183bc6eb2f8ec2a39f7d19f9d6267ec84/net/url_request/url_request_unittest.cc
[modify] https://crrev.com/230996f183bc6eb2f8ec2a39f7d19f9d6267ec84/services/network/BUILD.gn
[modify] https://crrev.com/230996f183bc6eb2f8ec2a39f7d19f9d6267ec84/services/network/network_context.cc
[modify] https://crrev.com/230996f183bc6eb2f8ec2a39f7d19f9d6267ec84/services/network/network_context.h
[modify] https://crrev.com/230996f183bc6eb2f8ec2a39f7d19f9d6267ec84/services/network/network_context_unittest.cc
[modify] https://crrev.com/230996f183bc6eb2f8ec2a39f7d19f9d6267ec84/services/network/public/mojom/network_context.mojom
[modify] https://crrev.com/230996f183bc6eb2f8ec2a39f7d19f9d6267ec84/services/network/public/mojom/network_service_test.mojom
[modify] https://crrev.com/230996f183bc6eb2f8ec2a39f7d19f9d6267ec84/services/network/test/test_network_context.h

Labels: -M-69 M-72
Status: Fixed (was: Started)
Summary: Deprecate And Remove header-based HTTP Public Key Pinning (HPKP) (was: Deprecate And Remove Public Key Pinning)
Discussed with palmer, we are going to use this bug just for the header-based public key pinning deprecation, as there are no immediate plans for static PKP.

One thing to note is the base dynamic key pinning code was left in place, with cronet as the only consumer. We can use  issue 890409  to discuss if that is a good long term solution for cronet.

See also issue 893055 about removing "Public-Key-Pins" from the list of non-cachable headers once we're sure the removal sticks.
Project Member

Comment 35 by bugdroid1@chromium.org, Oct 24

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

commit 1b467cd68d3281c5f9cc5c5c9e39e87bdf03902a
Author: Matt Mueller <mattm@chromium.org>
Date: Wed Oct 24 22:51:36 2018

Update TransportSecurityState::AddHPKP comment: dynamic PKP is not persisted.

PKP persistence was removed as part of https://crrev.com/230996f183bc6eb2f8ec2a39f7d19f9d6267ec84

Bug:  779166 
Change-Id: Ic8dcc6a983f08418465b301e99e0bd226521e073
Reviewed-on: https://chromium-review.googlesource.com/c/1297286
Reviewed-by: David Benjamin <davidben@chromium.org>
Commit-Queue: Matt Mueller <mattm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602508}
[modify] https://crrev.com/1b467cd68d3281c5f9cc5c5c9e39e87bdf03902a/net/http/transport_security_state.h

Project Member

Comment 36 by bugdroid1@chromium.org, Oct 24

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

commit 6eaede5f961d230d00bc6ba22f6de8b439524d61
Author: Matt Mueller <mattm@chromium.org>
Date: Wed Oct 24 23:21:33 2018

Remove TransportSecurityState::AddOrUpdateEnabledPKPHosts.

Unused since https://crrev.com/230996f183bc6eb2f8ec2a39f7d19f9d6267ec84

Bug:  779166 
Change-Id: I57e2f6253b834734829fd7acacc6254afbe07081
Reviewed-on: https://chromium-review.googlesource.com/c/1298418
Reviewed-by: David Benjamin <davidben@chromium.org>
Commit-Queue: Matt Mueller <mattm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602518}
[modify] https://crrev.com/6eaede5f961d230d00bc6ba22f6de8b439524d61/net/http/transport_security_state.cc
[modify] https://crrev.com/6eaede5f961d230d00bc6ba22f6de8b439524d61/net/http/transport_security_state.h

Sign in to add a comment