Issue 566144: Security panel: Indicate when HPKP was bypassed due to a local trust anchor

 Issue 565017  has been merged into this issue.

Hit this in the "wild", a comment in the console would have saved time.

https://projects.dm.id.lv/Public-Key-Pins_test was failing in recent and current Chrome 47 for Mac OS X

Failing because I had an alternative COMODO certificate for the same public key loaded into my keychain which had been loaded for some certificate related work with COMODO. Certificate attached so you have a really easy test case. This terminated the trust anchor chain locally rather than using the public chain, and thus resulted in HPKP checks being omitted, so no error is produced and my copy of Chrome is reported as not implementing HPKP (when the same version at BrowserStack on the same version of Mac OS X is correctly reported as supporting HPKP).

It took some investigation to realise it was a different chain of trust, and then a quick peak at the Chromium source code to see HPKP is suppressed when a local trust anchor is reached.

The RFC 7469 section 2.6 suggests it may be a "local policy", and whilst I hate the complexity, it feels like the kind of thing that might be better handled that way. Since presumably organisations who use an SSL proxy may want to ignore HPKP checks here, and not supply a useless error to console (assuming HPKP gets more popular). Although I believe most SSL proxies also suppress the headers, I'm sure someone will have laptops which are both proxied and not proxied - no matter what the security implications are).

Since R Sleevi is following I'll stop putting words into his mouth, since I'm sure he understands it far better than I do.

Certificate:
    Data:
        Version: 3 (0x2)
        Serial Number:
            4c:aa:f9:ca:db:63:6f:e0:1f:f7:4e:d8:5b:03:86:9d
        Signature Algorithm: sha384WithRSAEncryption
        Issuer: C=GB, ST=Greater Manchester, L=Salford, O=COMODO CA Limited, CN=COMODO RSA Certification Authority
        Validity
            Not Before: Jan 19 00:00:00 2010 GMT
            Not After : Jan 18 23:59:59 2038 GMT
        Subject: C=GB, ST=Greater Manchester, L=Salford, O=COMODO CA Limited, CN=COMODO RSA Certification Authority
        Subject Public Key Info:
            Public Key Algorithm: rsaEncryption
            RSA Public Key: (4096 bit)
                Modulus (4096 bit):
                    00:91:e8:54:92:d2:0a:56:b1:ac:0d:24:dd:c5:cf:
                    44:67:74:99:2b:37:a3:7d:23:70:00:71:bc:53:df:
                    c4:fa:2a:12:8f:4b:7f:10:56:bd:9f:70:72:b7:61:
                    7f:c9:4b:0f:17:a7:3d:e3:b0:04:61:ee:ff:11:97:
                    c7:f4:86:3e:0a:fa:3e:5c:f9:93:e6:34:7a:d9:14:
                    6b:e7:9c:b3:85:a0:82:7a:76:af:71:90:d7:ec:fd:
                    0d:fa:9c:6c:fa:df:b0:82:f4:14:7e:f9:be:c4:a6:
                    2f:4f:7f:99:7f:b5:fc:67:43:72:bd:0c:00:d6:89:
                    eb:6b:2c:d3:ed:8f:98:1c:14:ab:7e:e5:e3:6e:fc:
                    d8:a8:e4:92:24:da:43:6b:62:b8:55:fd:ea:c1:bc:
                    6c:b6:8b:f3:0e:8d:9a:e4:9b:6c:69:99:f8:78:48:
                    30:45:d5:ad:e1:0d:3c:45:60:fc:32:96:51:27:bc:
                    67:c3:ca:2e:b6:6b:ea:46:c7:c7:20:a0:b1:1f:65:
                    de:48:08:ba:a4:4e:a9:f2:83:46:37:84:eb:e8:cc:
                    81:48:43:67:4e:72:2a:9b:5c:bd:4c:1b:28:8a:5c:
                    22:7b:b4:ab:98:d9:ee:e0:51:83:c3:09:46:4e:6d:
                    3e:99:fa:95:17:da:7c:33:57:41:3c:8d:51:ed:0b:
                    b6:5c:af:2c:63:1a:df:57:c8:3f:bc:e9:5d:c4:9b:
                    af:45:99:e2:a3:5a:24:b4:ba:a9:56:3d:cf:6f:aa:
                    ff:49:58:be:f0:a8:ff:f4:b8:ad:e9:37:fb:ba:b8:
                    f4:0b:3a:f9:e8:43:42:1e:89:d8:84:cb:13:f1:d9:
                    bb:e1:89:60:b8:8c:28:56:ac:14:1d:9c:0a:e7:71:
                    eb:cf:0e:dd:3d:a9:96:a1:48:bd:3c:f7:af:b5:0d:
                    22:4c:c0:11:81:ec:56:3b:f6:d3:a2:e2:5b:b7:b2:
                    04:22:52:95:80:93:69:e8:8e:4c:65:f1:91:03:2d:
                    70:74:02:ea:8b:67:15:29:69:52:02:bb:d7:df:50:
                    6a:55:46:bf:a0:a3:28:61:7f:70:d0:c3:a2:aa:2c:
                    21:aa:47:ce:28:9c:06:45:76:bf:82:18:27:b4:d5:
                    ae:b4:cb:50:e6:6b:f4:4c:86:71:30:e9:a6:df:16:
                    86:e0:d8:ff:40:dd:fb:d0:42:88:7f:a3:33:3a:2e:
                    5c:1e:41:11:81:63:ce:18:71:6b:2b:ec:a6:8a:b7:
                    31:5c:3a:6a:47:e0:c3:79:59:d6:20:1a:af:f2:6a:
                    98:aa:72:bc:57:4a:d2:4b:9d:bb:10:fc:b0:4c:41:
                    e5:ed:1d:3d:5e:28:9d:9c:cc:bf:b3:51:da:a7:47:
                    e5:84:53
                Exponent: 65537 (0x10001)
        X509v3 extensions:
            X509v3 Subject Key Identifier: 
                BB:AF:7E:02:3D:FA:A6:F1:3C:84:8E:AD:EE:38:98:EC:D9:32:32:D4
            X509v3 Key Usage: critical
                Certificate Sign, CRL Sign
            X509v3 Basic Constraints: critical
                CA:TRUE
    Signature Algorithm: sha384WithRSAEncryption
        0a:f1:d5:46:84:b7:ae:51:bb:6c:b2:4d:41:14:00:93:4c:9c:
        cb:e5:c0:54:cf:a0:25:8e:02:f9:fd:b0:a2:0d:f5:20:98:3c:
        13:2d:ac:56:a2:b0:d6:7e:11:92:e9:2e:ba:9e:2e:9a:72:b1:
        bd:19:44:6c:61:35:a2:9a:b4:16:12:69:5a:8c:e1:d7:3e:a4:
        1a:e8:2f:03:f4:ae:61:1d:10:1b:2a:a4:8b:7a:c5:fe:05:a6:
        e1:c0:d6:c8:fe:9e:ae:8f:2b:ba:3d:99:f8:d8:73:09:58:46:
        6e:a6:9c:f4:d7:27:d3:95:da:37:83:72:1c:d3:73:e0:a2:47:
        99:03:38:5d:d5:49:79:00:29:1c:c7:ec:9b:20:1c:07:24:69:
        57:78:b2:39:fc:3a:84:a0:b5:9c:7c:8d:bf:2e:93:62:27:b7:
        39:da:17:18:ae:bd:3c:09:68:ff:84:9b:3c:d5:d6:0b:03:e3:
        57:9e:14:f7:d1:eb:4f:c8:bd:87:23:b7:b6:49:43:79:85:5c:
        ba:eb:92:0b:a1:c6:e8:68:a8:4c:16:b1:1a:99:0a:e8:53:2c:
        92:bb:a1:09:18:75:0c:65:a8:7b:cb:23:b7:1a:c2:28:85:c3:
        1b:ff:d0:2b:62:ef:a4:7b:09:91:98:67:8c:14:01:cd:68:06:
        6a:63:21:75:03:80:88:8a:6e:81:c6:85:f2:a9:a4:2d:e7:f4:
        a5:24:10:47:83:ca:cd:f4:8d:79:58:b1:06:9b:e7:1a:2a:d9:
        9d:01:d7:94:7d:ed:03:4a:ca:f0:db:e8:a9:01:3e:f5:56:99:
        c9:1e:8e:49:3d:bb:e5:09:b9:e0:4f:49:92:3d:16:82:40:cc:
        cc:59:c6:e6:3a:ed:12:2e:69:3c:6c:95:b1:fd:aa:1d:7b:7f:
        86:be:1e:0e:32:46:fb:fb:13:8f:75:7f:4c:8b:4b:46:63:fe:
        00:34:40:70:c1:c3:b9:a1:dd:a6:70:e2:04:b3:41:bc:e9:80:
        91:ea:64:9c:7a:e1:22:03:a9:9c:6e:6f:0e:65:4f:6c:87:87:
        5e:f3:6e:a0:f9:75:a5:9b:40:e8:53:b2:27:9d:4a:b9:c0:77:
        21:8d:ff:87:f2:de:bc:8c:ef:17:df:b7:49:0b:d1:f2:6e:30:
        0b:1a:0e:4e:76:ed:11:fc:f5:e9:56:b2:7d:bf:c7:6d:0a:93:
        8c:a5:d0:c0:b6:1d:be:3a:4e:94:a2:d7:6e:6c:0b:c2:8a:7c:
        fa:20:f3:c4:e4:e5:cd:0d:a8:cb:91:92:b1:7c:85:ec:b5:14:
        69:66:0e:82:e7:cd:ce:c8:2d:a6:51:7f:21:c1:35:53:85:06:
        4a:5d:9f:ad:bb:1b:5f:74

Deleted: COMODO RSA Certification Authority.cer 1.5 KB

COMODO RSA Certification Authority.cer 1.5 KB Download

How are people feeling about this proposal? That is, in devtools, showing a string whenever a locally installed root was used for a request, or specifically highlighting when pins were bypassed due to a locally installed root. (The latter would probably be a little trickier to implement.)

dadrian@ is interested in working on this, but I want to first make sure there's consensus that this is a passable idea. I personally think this information fits well in devtools; for example, a developer inspecting their certificate chain for problems should probably be alerted if they're actually inspecting a certificate generated by a MITM proxy.

I don't think that the cache segregation and UX issues described in previous bugs ( issue 565017 ,  issue 81623 ) are as applicable here, because we're not trying to synthesize an overall security state of a page that includes resources loaded with local roots; instead, we'd just be making neutral/factual statements about whether a given request was loaded with a public root or not.

My main concern is the reliability of SSLInfo::is_issued_by_known_root, since it can have false negatives and false positives on some platforms.

I think it makes sense to show such things for developers, yeah. If people are playing around and testing out HPKP behavior, they might want to know why it didn't behave as they expected.  Issue #469471  is a similar thing where invalid headers were confusing.

I kind of worry people will go nuts in thinking it's security UI, but maybe it'll instead shush those people so we can move on and do more useful things. :-)

> My main concern is the reliability of SSLInfo::is_issued_by_known_root, since it can have false negatives and false positives on some platforms.

It's certainly unreliable, though I think that's probably fine for diagnostic UI purposes? If the unreliability means a developer testing HPKP sees it not behaving as they expect, we should probably surface what factored into our behavior so they can debug.

Or did you have another issue in mind?

In terms of developer testing, I don't think that it matters that SSLInfo::is_issued_by_known_root isn't 100% reliable. Regardless of its reliability, the decision to bypass HPKP or not is dependent on the value of that variable. If you're a developer debugging your HPKP setup, you want to know if Chrome bypassed the HPKP setup, regardless of if Chrome's behavior was "correct".

Re #8 and 9, yeah, I agree that for the purposes of displaying when HPKP was bypassed, it's fine that it's unreliable. I was wondering if we would also want to just in general display when a local root is in use (independent of HPKP) -- for that, it's a little more worrisome that it's unreliable. So maybe we should just start with displaying when HPKP was bypassed.

Though, blegh, there is a little bit of a cache problem in that we won't be able to tell whether pins were bypassed for resources that were in the cache prior to these changes. I guess we could just live with that...?

I agree with #9. I think it would be good to surface this in Developer Tools.

Re #11: That is a bummer, but I think it at least gives developers a foothold for debugging. (Developers tend to know the importance of trying and re-trying after flushing the cache, too.)

Sounds alright to me.

Also note  Issue 587255 , which developers may also be interested in.

In answer to the question in 7, as someone who hit this in the wild I was specifically interested in when a pin was not checked, not generally spotting when a local certificate is used. That said either warning would have been useful, but I can imagine it might get annoying to always flag on local certificates.

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

commit df302c4d9de0bcd65378ecd44a4e3ec1199dea4c
Author: dadrian <dadrian@google.com>
Date: Fri Jun 10 18:48:59 2016

Add |pkp_bypassed| to SSLInfo.

This is the first step in exposing in devtools when pinning is bypassed due to a local trust anchor. To correctly expose this for all socket types, |pkp_bypassed| is also exposed in a CertificateVerifyResult, and set by each socket after calling TransportSecurityState::CheckPublicKeyPins.

This also updates HttpResponseInfo to serialize the new field correctly.

BUG= 566144 

Review-Url: https://codereview.chromium.org/2016143002
Cr-Commit-Position: refs/heads/master@{#399242}

[modify] https://crrev.com/df302c4d9de0bcd65378ecd44a4e3ec1199dea4c/net/http/http_response_info.cc
[modify] https://crrev.com/df302c4d9de0bcd65378ecd44a4e3ec1199dea4c/net/http/http_response_info_unittest.cc
[modify] https://crrev.com/df302c4d9de0bcd65378ecd44a4e3ec1199dea4c/net/http/transport_security_state.cc
[modify] https://crrev.com/df302c4d9de0bcd65378ecd44a4e3ec1199dea4c/net/http/transport_security_state.h
[modify] https://crrev.com/df302c4d9de0bcd65378ecd44a4e3ec1199dea4c/net/http/transport_security_state_unittest.cc
[modify] https://crrev.com/df302c4d9de0bcd65378ecd44a4e3ec1199dea4c/net/quic/crypto/proof_verifier_chromium.cc
[modify] https://crrev.com/df302c4d9de0bcd65378ecd44a4e3ec1199dea4c/net/quic/crypto/proof_verifier_chromium.h
[modify] https://crrev.com/df302c4d9de0bcd65378ecd44a4e3ec1199dea4c/net/quic/crypto/proof_verifier_chromium_test.cc
[modify] https://crrev.com/df302c4d9de0bcd65378ecd44a4e3ec1199dea4c/net/quic/quic_chromium_client_session.cc
[modify] https://crrev.com/df302c4d9de0bcd65378ecd44a4e3ec1199dea4c/net/quic/quic_chromium_client_session.h
[modify] https://crrev.com/df302c4d9de0bcd65378ecd44a4e3ec1199dea4c/net/socket/ssl_client_socket_impl.cc
[modify] https://crrev.com/df302c4d9de0bcd65378ecd44a4e3ec1199dea4c/net/socket/ssl_client_socket_impl.h
[modify] https://crrev.com/df302c4d9de0bcd65378ecd44a4e3ec1199dea4c/net/socket/ssl_client_socket_unittest.cc
[modify] https://crrev.com/df302c4d9de0bcd65378ecd44a4e3ec1199dea4c/net/spdy/spdy_session.cc
[modify] https://crrev.com/df302c4d9de0bcd65378ecd44a4e3ec1199dea4c/net/ssl/ssl_info.cc
[modify] https://crrev.com/df302c4d9de0bcd65378ecd44a4e3ec1199dea4c/net/ssl/ssl_info.h
[modify] https://crrev.com/df302c4d9de0bcd65378ecd44a4e3ec1199dea4c/net/url_request/url_request_unittest.cc

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

commit df302c4d9de0bcd65378ecd44a4e3ec1199dea4c
Author: dadrian <dadrian@google.com>
Date: Fri Jun 10 18:48:59 2016

Add |pkp_bypassed| to SSLInfo.

This is the first step in exposing in devtools when pinning is bypassed due to a local trust anchor. To correctly expose this for all socket types, |pkp_bypassed| is also exposed in a CertificateVerifyResult, and set by each socket after calling TransportSecurityState::CheckPublicKeyPins.

This also updates HttpResponseInfo to serialize the new field correctly.

BUG= 566144 

Review-Url: https://codereview.chromium.org/2016143002
Cr-Commit-Position: refs/heads/master@{#399242}

[modify] https://crrev.com/df302c4d9de0bcd65378ecd44a4e3ec1199dea4c/net/http/http_response_info.cc
[modify] https://crrev.com/df302c4d9de0bcd65378ecd44a4e3ec1199dea4c/net/http/http_response_info_unittest.cc
[modify] https://crrev.com/df302c4d9de0bcd65378ecd44a4e3ec1199dea4c/net/http/transport_security_state.cc
[modify] https://crrev.com/df302c4d9de0bcd65378ecd44a4e3ec1199dea4c/net/http/transport_security_state.h
[modify] https://crrev.com/df302c4d9de0bcd65378ecd44a4e3ec1199dea4c/net/http/transport_security_state_unittest.cc
[modify] https://crrev.com/df302c4d9de0bcd65378ecd44a4e3ec1199dea4c/net/quic/crypto/proof_verifier_chromium.cc
[modify] https://crrev.com/df302c4d9de0bcd65378ecd44a4e3ec1199dea4c/net/quic/crypto/proof_verifier_chromium.h
[modify] https://crrev.com/df302c4d9de0bcd65378ecd44a4e3ec1199dea4c/net/quic/crypto/proof_verifier_chromium_test.cc
[modify] https://crrev.com/df302c4d9de0bcd65378ecd44a4e3ec1199dea4c/net/quic/quic_chromium_client_session.cc
[modify] https://crrev.com/df302c4d9de0bcd65378ecd44a4e3ec1199dea4c/net/quic/quic_chromium_client_session.h
[modify] https://crrev.com/df302c4d9de0bcd65378ecd44a4e3ec1199dea4c/net/socket/ssl_client_socket_impl.cc
[modify] https://crrev.com/df302c4d9de0bcd65378ecd44a4e3ec1199dea4c/net/socket/ssl_client_socket_impl.h
[modify] https://crrev.com/df302c4d9de0bcd65378ecd44a4e3ec1199dea4c/net/socket/ssl_client_socket_unittest.cc
[modify] https://crrev.com/df302c4d9de0bcd65378ecd44a4e3ec1199dea4c/net/spdy/spdy_session.cc
[modify] https://crrev.com/df302c4d9de0bcd65378ecd44a4e3ec1199dea4c/net/ssl/ssl_info.cc
[modify] https://crrev.com/df302c4d9de0bcd65378ecd44a4e3ec1199dea4c/net/ssl/ssl_info.h
[modify] https://crrev.com/df302c4d9de0bcd65378ecd44a4e3ec1199dea4c/net/url_request/url_request_unittest.cc

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

commit bfa0a24b4ff377d337a704d01db5d5957b6eafa4
Author: dadrian <dadrian@google.com>
Date: Tue Jun 28 23:07:31 2016

Expose SSLInfo::pkp_bypassed to devtools security panel.

This adds the necessary plumbing to expose in devtools when public-key pinning is bypassed (due to a local trust anchor), by way of SecurityStyleExplanations. The goal is to allow the security panel to display when pins are bypassed, in order to help developers debug pinning implementations.

BUG= 566144 

Review-Url: https://codereview.chromium.org/2066483009
Cr-Commit-Position: refs/heads/master@{#402586}

[modify] https://crrev.com/bfa0a24b4ff377d337a704d01db5d5957b6eafa4/chrome/browser/ssl/chrome_security_state_model_client.cc
[modify] https://crrev.com/bfa0a24b4ff377d337a704d01db5d5957b6eafa4/chrome/browser/ssl/chrome_security_state_model_client_browser_tests.cc
[modify] https://crrev.com/bfa0a24b4ff377d337a704d01db5d5957b6eafa4/components/security_state/security_state_model.cc
[modify] https://crrev.com/bfa0a24b4ff377d337a704d01db5d5957b6eafa4/components/security_state/security_state_model.h
[modify] https://crrev.com/bfa0a24b4ff377d337a704d01db5d5957b6eafa4/content/common/ssl_status_serialization.cc
[modify] https://crrev.com/bfa0a24b4ff377d337a704d01db5d5957b6eafa4/content/public/browser/security_style_explanations.cc
[modify] https://crrev.com/bfa0a24b4ff377d337a704d01db5d5957b6eafa4/content/public/browser/security_style_explanations.h
[modify] https://crrev.com/bfa0a24b4ff377d337a704d01db5d5957b6eafa4/content/public/common/ssl_status.cc
[modify] https://crrev.com/bfa0a24b4ff377d337a704d01db5d5957b6eafa4/content/public/common/ssl_status.h

Photos for https://codereview.chromium.org/2118583003

Deleted: google-bypass.png 74.0 KB

	google-bypass.png 74.0 KB View Download

Deleted: github-bypass.png 440 KB

	github-bypass.png 440 KB View Download

Those screenshots in #20 basically show a lie.

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

commit 727e23f969ad3c54184efd67c6fae4bc7935b72a
Author: dadrian <dadrian@google.com>
Date: Thu Jul 07 22:17:53 2016

Display when PKP is bypassed in devtools

This adds a security explanation to devtools for when public-key pinning is bypassed due to a local trust anchor. It does not affect the security state of the page, and enforces that "Info" explanations are shown last.

BUG= 566144 

Review-Url: https://codereview.chromium.org/2118583003
Cr-Commit-Position: refs/heads/master@{#404253}

[modify] https://crrev.com/727e23f969ad3c54184efd67c6fae4bc7935b72a/chrome/browser/ssl/chrome_security_state_model_client.cc
[modify] https://crrev.com/727e23f969ad3c54184efd67c6fae4bc7935b72a/chrome/browser/ssl/chrome_security_state_model_client_browser_tests.cc
[modify] https://crrev.com/727e23f969ad3c54184efd67c6fae4bc7935b72a/content/browser/devtools/protocol/security_handler.cc
[modify] https://crrev.com/727e23f969ad3c54184efd67c6fae4bc7935b72a/content/public/browser/security_style_explanations.h
[add] https://crrev.com/727e23f969ad3c54184efd67c6fae4bc7935b72a/third_party/WebKit/LayoutTests/http/tests/inspector/security/security-explanation-ordering-expected.txt
[add] https://crrev.com/727e23f969ad3c54184efd67c6fae4bc7935b72a/third_party/WebKit/LayoutTests/http/tests/inspector/security/security-explanation-ordering.html
[modify] https://crrev.com/727e23f969ad3c54184efd67c6fae4bc7935b72a/third_party/WebKit/Source/devtools/front_end/security/SecurityPanel.js
[modify] https://crrev.com/727e23f969ad3c54184efd67c6fae4bc7935b72a/third_party/WebKit/Source/devtools/front_end/security/mainView.css