Project: chromium Issues People Development process History Sign in
New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 10 users
Status: Fixed
Owner:
OOO through Oct 22
Closed: Aug 2015
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Feature



Sign in to add a comment
HPKP Reporting on invalid pins
Reported by craig.fr...@gmail.com, Jan 1 2015 Back to list
UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_9_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/39.0.2171.95 Safari/537.36

Steps to reproduce the problem:
Setup a HTTPS website and include the following header (whitespace added here to make it easier to read):

    Public-Key-Pins-Report-Only: 
        pin-sha256="bRmMf0OkJ8ArV9VPmDsSFeK253UBjMBVo5t8VmdY4Lw=";
        pin-sha256="7fPFjIXIMozawdIR/Ue7AjOusulKX6Q+4hqdhazjr9E=";
        max-age=2592000;
        includeSubDomains;
        report-uri="https://www.example.com/pkp-report"

Note that the sha256 values are intentionally incorrect (e.g. imagine a developer doing a copy/paste).

The same behaviour can be seen with the "Public-Key-Pins" header as well.

What is the expected behavior?
Some kind of error in the dev console... or some attempt to send a report to the report-uri (in this case the report-uri was on the same domain, which will be an issue, although one suggestion was to cache the reports and send when the fingerprint is valid - something that does not appear to happen at the moment).

What went wrong?
Nothing happens... the header is completely ignored in this situation.

Did this work before? No 

Chrome version: 39.0.2171.95  Channel: stable
OS Version: OS X 10.9.5
Flash Version: Shockwave Flash 16.0 r0

Setting a valid sha256 fingerprint does cause the entry to be recorded and appear in:

chrome://net-internals/#hsts

But developers will need some diagnostic information to help them test and implement this header.
 
Comment 1 by f...@chromium.org, Jan 1 2015
Cc: rsleevi@chromium.org lgar...@chromium.org
Labels: -Restrict-View-SecurityTeam -Type-Bug-Security Type-Feature Cr-Internals-Network-SSL
Owner: palmer@chromium.org
Status: Untriaged
Comment 2 by palmer@google.com, Jan 2 2015
Labels: -OS-Mac OS-All Cr-Security
Status: Assigned
Correct, we have not yet implemented support for report-uri (and -report-only, for that matter). Those features were added to the specification late in the process. We should add it, but I am not sure when I am going to have time. If anyone else is excited to take this bug up, please feel free.

In addition to implementing sending reports, we should add a warning to the dev tools console, and write a message to the dev tools console about what kind of response the report request got back (200, 5xx, 4xx, et c.).
Fair enough... adding a report-uri can be tricky (especially on the same domain, assuming an invalid pin).

But can I also link to  issue 445359  (adding a Security tab to the dev tools):

https://code.google.com/p/chromium/issues/detail?id=445359

I think that will really help developers be aware of this new feature, help with implementation, and show that it's working correctly (the console still needs to show errors though).
Cc: palmer@chromium.org davidben@chromium.org
Owner: ----
Status: Available
I'm not going to get to this until Q2 2015 at the earliest. If anyone else wants to pick this up, that'd be great.
Comment 5 by est...@chromium.org, Mar 23 2015
Owner: est...@chromium.org
Status: Assigned
I'm going to start looking into this, since it dovetails nicely with the invalid cert reporter (issue 461817).
Project Member Comment 6 by bugdroid1@chromium.org, Jul 21 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1320e36d908427d615357df1630348bfb38cb5c4

commit 1320e36d908427d615357df1630348bfb38cb5c4
Author: estark <estark@chromium.org>
Date: Tue Jul 21 16:39:41 2015

Parse HPKP report-uri and persist in TransportSecurityPersister

This CL parses the report-uri attribute on HPKP headers and stores them
in TransportSecurityPersister.

This is CL #1.
CL #2: crrev.com/1212973002 (add net::CertificateReportSender)
CL #3: crrev.com/1212613004 (add net::TransportSecurityReporter)
CL #4: crrev.com/1213783005 (send HPKP reports)

BUG= 445793 

Review URL: https://codereview.chromium.org/1211363005

Cr-Commit-Position: refs/heads/master@{#339667}

[modify] http://crrev.com/1320e36d908427d615357df1630348bfb38cb5c4/chrome/browser/ui/webui/net_internals/net_internals_ui.cc
[modify] http://crrev.com/1320e36d908427d615357df1630348bfb38cb5c4/net/http/http_security_headers.cc
[modify] http://crrev.com/1320e36d908427d615357df1630348bfb38cb5c4/net/http/http_security_headers.h
[modify] http://crrev.com/1320e36d908427d615357df1630348bfb38cb5c4/net/http/http_security_headers_unittest.cc
[modify] http://crrev.com/1320e36d908427d615357df1630348bfb38cb5c4/net/http/http_util.cc
[modify] http://crrev.com/1320e36d908427d615357df1630348bfb38cb5c4/net/http/http_util.h
[modify] http://crrev.com/1320e36d908427d615357df1630348bfb38cb5c4/net/http/http_util_unittest.cc
[modify] http://crrev.com/1320e36d908427d615357df1630348bfb38cb5c4/net/http/transport_security_persister.cc
[modify] http://crrev.com/1320e36d908427d615357df1630348bfb38cb5c4/net/http/transport_security_persister_unittest.cc
[modify] http://crrev.com/1320e36d908427d615357df1630348bfb38cb5c4/net/http/transport_security_state.cc
[modify] http://crrev.com/1320e36d908427d615357df1630348bfb38cb5c4/net/http/transport_security_state.h
[modify] http://crrev.com/1320e36d908427d615357df1630348bfb38cb5c4/net/http/transport_security_state_unittest.cc

Project Member Comment 7 by bugdroid1@chromium.org, Jul 21 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ebc4df76fef37fb51415c23d84af590e48a93069

commit ebc4df76fef37fb51415c23d84af590e48a93069
Author: rsleevi <rsleevi@chromium.org>
Date: Tue Jul 21 22:05:16 2015

Revert of Parse HPKP report-uri and persist in TransportSecurityPersister (patchset #11 id:200001 of https://codereview.chromium.org/1211363005/)

Reason for revert:
Causes Win7 to hang

Original issue's description:
> Parse HPKP report-uri and persist in TransportSecurityPersister
>
> This CL parses the report-uri attribute on HPKP headers and stores them
> in TransportSecurityPersister.
>
> This is CL #1.
> CL #2: crrev.com/1212973002 (add net::CertificateReportSender)
> CL #3: crrev.com/1212613004 (add net::TransportSecurityReporter)
> CL #4: crrev.com/1213783005 (send HPKP reports)
>
> BUG= 445793 
>
> Committed: https://crrev.com/1320e36d908427d615357df1630348bfb38cb5c4
> Cr-Commit-Position: refs/heads/master@{#339667}

TBR=davidben@chromium.org,eroman@chromium.org,estark@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 445793 

Review URL: https://codereview.chromium.org/1249823002

Cr-Commit-Position: refs/heads/master@{#339759}

[modify] http://crrev.com/ebc4df76fef37fb51415c23d84af590e48a93069/chrome/browser/ui/webui/net_internals/net_internals_ui.cc
[modify] http://crrev.com/ebc4df76fef37fb51415c23d84af590e48a93069/net/http/http_security_headers.cc
[modify] http://crrev.com/ebc4df76fef37fb51415c23d84af590e48a93069/net/http/http_security_headers.h
[modify] http://crrev.com/ebc4df76fef37fb51415c23d84af590e48a93069/net/http/http_security_headers_unittest.cc
[modify] http://crrev.com/ebc4df76fef37fb51415c23d84af590e48a93069/net/http/http_util.cc
[modify] http://crrev.com/ebc4df76fef37fb51415c23d84af590e48a93069/net/http/http_util.h
[modify] http://crrev.com/ebc4df76fef37fb51415c23d84af590e48a93069/net/http/http_util_unittest.cc
[modify] http://crrev.com/ebc4df76fef37fb51415c23d84af590e48a93069/net/http/transport_security_persister.cc
[modify] http://crrev.com/ebc4df76fef37fb51415c23d84af590e48a93069/net/http/transport_security_persister_unittest.cc
[modify] http://crrev.com/ebc4df76fef37fb51415c23d84af590e48a93069/net/http/transport_security_state.cc
[modify] http://crrev.com/ebc4df76fef37fb51415c23d84af590e48a93069/net/http/transport_security_state.h
[modify] http://crrev.com/ebc4df76fef37fb51415c23d84af590e48a93069/net/http/transport_security_state_unittest.cc

Project Member Comment 8 by bugdroid1@chromium.org, Jul 27 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/83487b6773bbda6a46191dede6010ec90e5a9ae8

commit 83487b6773bbda6a46191dede6010ec90e5a9ae8
Author: estark <estark@chromium.org>
Date: Mon Jul 27 17:11:14 2015

Parse HPKP report-uri and persist in TransportSecurityPersister

This CL parses the report-uri attribute on HPKP headers and stores them
in TransportSecurityPersister.

This is CL #1.
CL #2: crrev.com/1212973002 (add net::CertificateReportSender)
CL #3: crrev.com/1212613004 (build and send HPKP reports)

BUG= 445793 

Committed: https://crrev.com/1320e36d908427d615357df1630348bfb38cb5c4
Cr-Commit-Position: refs/heads/master@{#339667}

Review URL: https://codereview.chromium.org/1211363005

Cr-Commit-Position: refs/heads/master@{#340490}

[modify] http://crrev.com/83487b6773bbda6a46191dede6010ec90e5a9ae8/chrome/browser/ui/webui/net_internals/net_internals_ui.cc
[modify] http://crrev.com/83487b6773bbda6a46191dede6010ec90e5a9ae8/net/http/http_security_headers.cc
[modify] http://crrev.com/83487b6773bbda6a46191dede6010ec90e5a9ae8/net/http/http_security_headers.h
[modify] http://crrev.com/83487b6773bbda6a46191dede6010ec90e5a9ae8/net/http/http_security_headers_unittest.cc
[modify] http://crrev.com/83487b6773bbda6a46191dede6010ec90e5a9ae8/net/http/http_util.cc
[modify] http://crrev.com/83487b6773bbda6a46191dede6010ec90e5a9ae8/net/http/http_util.h
[modify] http://crrev.com/83487b6773bbda6a46191dede6010ec90e5a9ae8/net/http/http_util_unittest.cc
[modify] http://crrev.com/83487b6773bbda6a46191dede6010ec90e5a9ae8/net/http/transport_security_persister.cc
[modify] http://crrev.com/83487b6773bbda6a46191dede6010ec90e5a9ae8/net/http/transport_security_persister_unittest.cc
[modify] http://crrev.com/83487b6773bbda6a46191dede6010ec90e5a9ae8/net/http/transport_security_state.cc
[modify] http://crrev.com/83487b6773bbda6a46191dede6010ec90e5a9ae8/net/http/transport_security_state.h
[modify] http://crrev.com/83487b6773bbda6a46191dede6010ec90e5a9ae8/net/http/transport_security_state_unittest.cc

Project Member Comment 9 by bugdroid1@chromium.org, Jul 28 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/19be6db116a497bc311a7faf80279a1233eb7ace

commit 19be6db116a497bc311a7faf80279a1233eb7ace
Author: estark <estark@chromium.org>
Date: Tue Jul 28 05:16:47 2015

Add net::CertificateReportSender for handling cert report sending

net::CertificateReportSender contains code factored out of
CertificateErrorReporter in //chrome. net::CertificateReportSender will
be used for both HPKP violation reports and the Safe Browsing Extended
Reporting cert reports that CertificateErrorReporter handles.

CL #1: crrev.com/1211363005 (parse report-uri)
This is CL #2.
CL #3: crrev.com/1212613004 (build and send HPKP reports)

BUG= 445793 

Review URL: https://codereview.chromium.org/1212973002

Cr-Commit-Position: refs/heads/master@{#340641}

[modify] http://crrev.com/19be6db116a497bc311a7faf80279a1233eb7ace/chrome/browser/net/certificate_error_reporter.cc
[modify] http://crrev.com/19be6db116a497bc311a7faf80279a1233eb7ace/chrome/browser/net/certificate_error_reporter.h
[modify] http://crrev.com/19be6db116a497bc311a7faf80279a1233eb7ace/chrome/browser/net/certificate_error_reporter_unittest.cc
[modify] http://crrev.com/19be6db116a497bc311a7faf80279a1233eb7ace/chrome/browser/safe_browsing/ping_manager.cc
[modify] http://crrev.com/19be6db116a497bc311a7faf80279a1233eb7ace/chrome/browser/ssl/certificate_reporting_test_utils.cc
[modify] http://crrev.com/19be6db116a497bc311a7faf80279a1233eb7ace/chrome/browser/ssl/chrome_fraudulent_certificate_reporter.cc
[modify] http://crrev.com/19be6db116a497bc311a7faf80279a1233eb7ace/chrome/browser/ssl/chrome_fraudulent_certificate_reporter_unittest.cc
[modify] http://crrev.com/19be6db116a497bc311a7faf80279a1233eb7ace/net/http/transport_security_state.h
[modify] http://crrev.com/19be6db116a497bc311a7faf80279a1233eb7ace/net/net.gypi
[add] http://crrev.com/19be6db116a497bc311a7faf80279a1233eb7ace/net/url_request/certificate_report_sender.cc
[add] http://crrev.com/19be6db116a497bc311a7faf80279a1233eb7ace/net/url_request/certificate_report_sender.h
[add] http://crrev.com/19be6db116a497bc311a7faf80279a1233eb7ace/net/url_request/certificate_report_sender_unittest.cc

Project Member Comment 10 by bugdroid1@chromium.org, Jul 28 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1a66df754cd86315086180e978298f453f8723e7

commit 1a66df754cd86315086180e978298f453f8723e7
Author: estark <estark@chromium.org>
Date: Tue Jul 28 15:24:00 2015

Build and send HPKP violation reports

This CL adds code to TransportSecurityState to build HPKP reports, and
sends them with a CertificateReportSender constructed by
ProfileIOData. Calls to CheckPublicKeyPins() indicate whether a report
should be sent and pass necessary reporting information as arguments.

CL #1: crrev.com/1211363005 (parse report-uri)
CL #2: crrev.com/1212973002 (add net::CertificateReportSender)
This is CL #3.

BUG= 445793 

Review URL: https://codereview.chromium.org/1212613004

Cr-Commit-Position: refs/heads/master@{#340687}

[modify] http://crrev.com/1a66df754cd86315086180e978298f453f8723e7/chrome/browser/profiles/profile_io_data.cc
[modify] http://crrev.com/1a66df754cd86315086180e978298f453f8723e7/chrome/browser/profiles/profile_io_data.h
[modify] http://crrev.com/1a66df754cd86315086180e978298f453f8723e7/net/base/hash_value.h
[modify] http://crrev.com/1a66df754cd86315086180e978298f453f8723e7/net/http/http_security_headers_unittest.cc
[modify] http://crrev.com/1a66df754cd86315086180e978298f453f8723e7/net/http/transport_security_state.cc
[modify] http://crrev.com/1a66df754cd86315086180e978298f453f8723e7/net/http/transport_security_state.h
[modify] http://crrev.com/1a66df754cd86315086180e978298f453f8723e7/net/http/transport_security_state_unittest.cc
[modify] http://crrev.com/1a66df754cd86315086180e978298f453f8723e7/net/quic/crypto/proof_verifier_chromium.cc
[modify] http://crrev.com/1a66df754cd86315086180e978298f453f8723e7/net/socket/ssl_client_socket_nss.cc
[modify] http://crrev.com/1a66df754cd86315086180e978298f453f8723e7/net/socket/ssl_client_socket_openssl.cc
[modify] http://crrev.com/1a66df754cd86315086180e978298f453f8723e7/net/spdy/spdy_session.cc

Project Member Comment 11 by bugdroid1@chromium.org, Aug 1 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/640590d4170cd3d2ac9dec78d655ef064ec1b4a0

commit 640590d4170cd3d2ac9dec78d655ef064ec1b4a0
Author: estark <estark@chromium.org>
Date: Fri Jul 31 23:56:24 2015

Add parsing for Public-Key-Pins-Report-Only header

Adds the ability to parse HPKP Report-Only headers, which will later be
processed on the requests on which they are received.

BUG= 445793 

Review URL: https://codereview.chromium.org/1267513003

Cr-Commit-Position: refs/heads/master@{#341441}

[modify] http://crrev.com/640590d4170cd3d2ac9dec78d655ef064ec1b4a0/net/http/http_security_headers.cc
[modify] http://crrev.com/640590d4170cd3d2ac9dec78d655ef064ec1b4a0/net/http/http_security_headers.h
[modify] http://crrev.com/640590d4170cd3d2ac9dec78d655ef064ec1b4a0/net/http/http_security_headers_unittest.cc

Project Member Comment 13 by bugdroid1@chromium.org, Aug 7 2015
Project Member Comment 14 by bugdroid1@chromium.org, Aug 8 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b6abb75f583880f97751784b8aaa02e53214744f

commit b6abb75f583880f97751784b8aaa02e53214744f
Author: agl <agl@chromium.org>
Date: Sat Aug 08 01:06:16 2015

net: update generated HSTS lists.

This change should be a no-op as it just regenerates the HSTS data in
light of [1]. This causes empty report URIs to be added to each pinset.

[1] https://github.com/agl/transport-security-state-generate/commit/4ab5faf44f8cc090525b3609dab698064b77154d.

BUG= 445793 

Review URL: https://codereview.chromium.org/1282003004

Cr-Commit-Position: refs/heads/master@{#342497}

[modify] http://crrev.com/b6abb75f583880f97751784b8aaa02e53214744f/net/http/transport_security_state_static.h

Project Member Comment 15 by bugdroid1@chromium.org, Aug 12 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/db949c345a8c561f45a2351daa06dc9c85671e88

commit db949c345a8c561f45a2351daa06dc9c85671e88
Author: estark <estark@chromium.org>
Date: Wed Aug 12 02:35:50 2015

Allow preloaded pins to contain report URIs; remove special-case reporting

This CL processes report URIs in preloaded pins and removes special-case code
for reporting pin violations on Google properties
(FraudulentCertificateReporter and its implementation
ChromeFraudulentCertificateReporter), in favor of a preloaded report
URI.

BUG= 445793 

Review URL: https://codereview.chromium.org/1267383002

Cr-Commit-Position: refs/heads/master@{#342967}

[modify] http://crrev.com/db949c345a8c561f45a2351daa06dc9c85671e88/chrome/browser/net/certificate_error_reporter.cc
[modify] http://crrev.com/db949c345a8c561f45a2351daa06dc9c85671e88/chrome/browser/net/certificate_error_reporter.h
[modify] http://crrev.com/db949c345a8c561f45a2351daa06dc9c85671e88/chrome/browser/net/certificate_error_reporter_unittest.cc
[modify] http://crrev.com/db949c345a8c561f45a2351daa06dc9c85671e88/chrome/browser/profiles/off_the_record_profile_io_data.cc
[modify] http://crrev.com/db949c345a8c561f45a2351daa06dc9c85671e88/chrome/browser/profiles/profile_impl_io_data.cc
[modify] http://crrev.com/db949c345a8c561f45a2351daa06dc9c85671e88/chrome/browser/profiles/profile_io_data.cc
[modify] http://crrev.com/db949c345a8c561f45a2351daa06dc9c85671e88/chrome/browser/profiles/profile_io_data.h
[delete] http://crrev.com/20162d198ff2e82c959c133801a5ae47c4470c6e/chrome/browser/ssl/chrome_fraudulent_certificate_reporter.cc
[delete] http://crrev.com/20162d198ff2e82c959c133801a5ae47c4470c6e/chrome/browser/ssl/chrome_fraudulent_certificate_reporter.h
[delete] http://crrev.com/20162d198ff2e82c959c133801a5ae47c4470c6e/chrome/browser/ssl/chrome_fraudulent_certificate_reporter_unittest.cc
[modify] http://crrev.com/db949c345a8c561f45a2351daa06dc9c85671e88/chrome/chrome_browser.gypi
[modify] http://crrev.com/db949c345a8c561f45a2351daa06dc9c85671e88/chrome/chrome_tests_unit.gypi
[modify] http://crrev.com/db949c345a8c561f45a2351daa06dc9c85671e88/net/http/transport_security_state.cc
[modify] http://crrev.com/db949c345a8c561f45a2351daa06dc9c85671e88/net/http/transport_security_state_static.h
[modify] http://crrev.com/db949c345a8c561f45a2351daa06dc9c85671e88/net/http/transport_security_state_static.json
[modify] http://crrev.com/db949c345a8c561f45a2351daa06dc9c85671e88/net/http/transport_security_state_unittest.cc
[modify] http://crrev.com/db949c345a8c561f45a2351daa06dc9c85671e88/net/net.gypi
[modify] http://crrev.com/db949c345a8c561f45a2351daa06dc9c85671e88/net/url_request/certificate_report_sender.cc
[modify] http://crrev.com/db949c345a8c561f45a2351daa06dc9c85671e88/net/url_request/certificate_report_sender.h
[delete] http://crrev.com/20162d198ff2e82c959c133801a5ae47c4470c6e/net/url_request/fraudulent_certificate_reporter.h
[modify] http://crrev.com/db949c345a8c561f45a2351daa06dc9c85671e88/net/url_request/url_request_context.cc
[modify] http://crrev.com/db949c345a8c561f45a2351daa06dc9c85671e88/net/url_request/url_request_context.h
[modify] http://crrev.com/db949c345a8c561f45a2351daa06dc9c85671e88/net/url_request/url_request_context_storage.cc
[modify] http://crrev.com/db949c345a8c561f45a2351daa06dc9c85671e88/net/url_request/url_request_context_storage.h
[modify] http://crrev.com/db949c345a8c561f45a2351daa06dc9c85671e88/net/url_request/url_request_http_job.cc

Status: Fixed
Some follow-up work to be done in Issue 516910.
Also, more follow-up in  issue 522332 .
Labels: M-46
Sign in to add a comment