Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(40)

Issue 1652603002: Add information to SSLInfo about CT EV policy compliance (Closed)

Created:
4 years, 10 months ago by estark
Modified:
4 years, 10 months ago
CC:
chromium-reviews, certificate-transparency-chrome_googlegroups.com, cbentzel+watch_chromium.org, Eran Messeri
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Add information to SSLInfo about CT EV policy compliance This CL adds a field to SSLInfo to record whether CT policies were enforced on the connection and details about the connection's compliance with the CT EV policy. This will eventually allow UI to explain to domain owners why their site's EV status might be getting stripped. This also lays the groundwork for introducing an Expect-CT policy, which will be applied on all certificates. //net will apply the expect CT policy and export the result via the new field in SSLInfo, so that code outside net can send a report if desired. BUG=568806 Committed: https://crrev.com/723b5eeb4486ac293b6574cfce33a4fb1012e09d Cr-Commit-Position: refs/heads/master@{#376256}

Patch Set 1 #

Patch Set 2 : update comments #

Patch Set 3 : expand a comment #

Total comments: 11

Patch Set 4 : rsleevi comments #

Patch Set 5 : SSLClientSOcket nss fix #

Total comments: 5

Patch Set 6 : move EVPolicyCompliance to separate header #

Patch Set 7 : add TODO for CTVerifyResult in CTPolicyEnforcer tests #

Total comments: 4

Patch Set 8 : rsleevi comments #

Patch Set 9 : some cleanup #

Total comments: 18

Patch Set 10 : rsleevi nits #

Unified diffs Side-by-side diffs Delta from patch set Stats (+280 lines, -130 lines) Patch
M net/cert/ct_policy_enforcer.h View 1 2 3 4 5 6 7 8 9 2 chunks +17 lines, -11 lines 0 comments Download
M net/cert/ct_policy_enforcer.cc View 1 2 3 4 5 6 7 8 9 9 chunks +40 lines, -46 lines 0 comments Download
M net/cert/ct_policy_enforcer_unittest.cc View 1 2 3 4 5 6 7 8 9 11 chunks +51 lines, -28 lines 0 comments Download
A net/cert/ct_policy_status.h View 1 2 3 4 5 6 7 8 9 1 chunk +39 lines, -0 lines 0 comments Download
M net/cert/ct_verify_result.h View 1 2 3 4 5 6 7 8 9 2 chunks +12 lines, -3 lines 0 comments Download
M net/cert/ct_verify_result.cc View 1 2 3 4 5 6 7 8 9 1 chunk +5 lines, -1 line 0 comments Download
M net/net.gypi View 1 2 3 4 5 1 chunk +1 line, -0 lines 0 comments Download
M net/quic/crypto/proof_verifier_chromium.cc View 1 2 3 4 5 6 7 8 9 2 chunks +16 lines, -2 lines 0 comments Download
M net/quic/crypto/proof_verifier_chromium_test.cc View 1 2 3 4 5 6 7 8 9 3 chunks +14 lines, -10 lines 0 comments Download
M net/quic/quic_chromium_client_session.cc View 1 chunk +1 line, -1 line 0 comments Download
M net/socket/ssl_client_socket_nss.h View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M net/socket/ssl_client_socket_nss.cc View 1 2 3 4 5 6 7 8 9 4 chunks +17 lines, -5 lines 0 comments Download
M net/socket/ssl_client_socket_openssl.h View 1 chunk +3 lines, -2 lines 0 comments Download
M net/socket/ssl_client_socket_openssl.cc View 1 2 3 4 5 6 7 8 9 4 chunks +17 lines, -5 lines 0 comments Download
M net/socket/ssl_client_socket_unittest.cc View 1 2 3 4 5 6 7 8 9 4 chunks +9 lines, -6 lines 0 comments Download
M net/ssl/ssl_info.h View 1 2 3 4 5 6 7 8 9 3 chunks +26 lines, -6 lines 0 comments Download
M net/ssl/ssl_info.cc View 1 2 3 4 5 6 7 8 9 4 chunks +11 lines, -3 lines 0 comments Download

Messages

Total messages: 29 (8 generated)
estark
Ryan, Eran: This is one possible approach for exposing details about CT policy compliance outside ...
4 years, 10 months ago (2016-01-30 17:03:29 UTC) #3
Ryan Sleevi
Apologies for the delay. I have some design questions, which I think would really benefit ...
4 years, 10 months ago (2016-02-05 02:09:25 UTC) #5
estark
Thanks for the comments, Ryan. No code changes yet -- I see what you mean ...
4 years, 10 months ago (2016-02-07 22:37:36 UTC) #6
Ryan Sleevi
On 2016/02/07 22:37:36, estark wrote: > Another possible approach might be to punt on this ...
4 years, 10 months ago (2016-02-07 22:54:10 UTC) #7
estark
On 2016/02/07 22:54:10, Ryan Sleevi wrote: > On 2016/02/07 22:37:36, estark wrote: > > Another ...
4 years, 10 months ago (2016-02-08 08:36:09 UTC) #8
estark
https://codereview.chromium.org/1652603002/diff/40001/net/cert/ct_policy_enforcer.h File net/cert/ct_policy_enforcer.h (right): https://codereview.chromium.org/1652603002/diff/40001/net/cert/ct_policy_enforcer.h#newcode63 net/cert/ct_policy_enforcer.h:63: verified_scts, On 2016/02/05 02:09:25, Ryan Sleevi wrote: > On ...
4 years, 10 months ago (2016-02-08 08:36:26 UTC) #9
estark
Oh, sorry, just wanted to clarify: I'll give Opera a couple days to look at ...
4 years, 10 months ago (2016-02-08 08:37:42 UTC) #10
haavardm
https://codereview.chromium.org/1652603002/diff/40001/net/cert/ct_policy_enforcer.h File net/cert/ct_policy_enforcer.h (right): https://codereview.chromium.org/1652603002/diff/40001/net/cert/ct_policy_enforcer.h#newcode48 net/cert/ct_policy_enforcer.h:48: }; On 2016/02/05 02:09:25, Ryan Sleevi wrote: > The ...
4 years, 10 months ago (2016-02-08 12:49:15 UTC) #11
Eran Messeri
Looks good, with one minor reservation. Re necessity of fine-grained details & enum to expose ...
4 years, 10 months ago (2016-02-10 17:41:09 UTC) #12
Ryan Sleevi
https://codereview.chromium.org/1652603002/diff/80001/net/cert/ct_verify_result.h File net/cert/ct_verify_result.h (right): https://codereview.chromium.org/1652603002/diff/80001/net/cert/ct_verify_result.h#newcode37 net/cert/ct_verify_result.h:37: CTPolicyEnforcer::EVPolicyCompliance ev_policy_compliance; On 2016/02/10 17:41:09, Eran Messeri wrote: > ...
4 years, 10 months ago (2016-02-10 17:57:06 UTC) #13
estark
Thanks everyone for the input so far. So it sounds like this design is workable ...
4 years, 10 months ago (2016-02-11 01:54:21 UTC) #14
Ryan Sleevi
https://codereview.chromium.org/1652603002/diff/120001/net/quic/crypto/proof_verifier_chromium.cc File net/quic/crypto/proof_verifier_chromium.cc (right): https://codereview.chromium.org/1652603002/diff/120001/net/quic/crypto/proof_verifier_chromium.cc#newcode302 net/quic/crypto/proof_verifier_chromium.cc:302: ev_policy_compliance != ct::EV_POLICY_COMPLIES_VIA_SCTS) { Why does DOES_NOT_APPLY not need ...
4 years, 10 months ago (2016-02-11 02:57:24 UTC) #15
estark
https://codereview.chromium.org/1652603002/diff/120001/net/quic/crypto/proof_verifier_chromium.cc File net/quic/crypto/proof_verifier_chromium.cc (right): https://codereview.chromium.org/1652603002/diff/120001/net/quic/crypto/proof_verifier_chromium.cc#newcode302 net/quic/crypto/proof_verifier_chromium.cc:302: ev_policy_compliance != ct::EV_POLICY_COMPLIES_VIA_SCTS) { On 2016/02/11 02:57:24, Ryan Sleevi ...
4 years, 10 months ago (2016-02-11 04:06:20 UTC) #16
Eran Messeri
lgtm
4 years, 10 months ago (2016-02-11 06:17:23 UTC) #17
estark
friendly ping to Ryan... unless perhaps there's someone you'd suggest for reviewing expect-CT CLs while ...
4 years, 10 months ago (2016-02-13 00:48:54 UTC) #18
estark
Hi David -- do you think you might be able to review this while Ryan ...
4 years, 10 months ago (2016-02-18 05:53:08 UTC) #20
Ryan Sleevi
LGTM with a variety of nits, but no reason to hold up for a re-review. ...
4 years, 10 months ago (2016-02-18 06:46:52 UTC) #21
estark
Thanks Ryan! https://codereview.chromium.org/1652603002/diff/160001/net/cert/ct_policy_enforcer.h File net/cert/ct_policy_enforcer.h (right): https://codereview.chromium.org/1652603002/diff/160001/net/cert/ct_policy_enforcer.h#newcode4 net/cert/ct_policy_enforcer.h:4: #ifndef NET_CERT_CT_POLICY_ENFORCER_H On 2016/02/18 06:46:51, Ryan Sleevi ...
4 years, 10 months ago (2016-02-18 19:24:32 UTC) #22
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1652603002/180001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1652603002/180001
4 years, 10 months ago (2016-02-18 19:26:50 UTC) #25
commit-bot: I haz the power
Committed patchset #10 (id:180001)
4 years, 10 months ago (2016-02-18 21:01:21 UTC) #27
commit-bot: I haz the power
4 years, 10 months ago (2016-02-18 21:02:43 UTC) #29
Message was sent while issue was closed.
Patchset 10 (id:??) landed as
https://crrev.com/723b5eeb4486ac293b6574cfce33a4fb1012e09d
Cr-Commit-Position: refs/heads/master@{#376256}

Powered by Google App Engine
This is Rietveld 408576698