Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(2)

Issue 462543002: Certificate Transparency: Code for unpacking EV cert hashes whitelist (Closed)

Created:
6 years, 5 months ago by Eran Messeri
Modified:
6 years, 4 months ago
CC:
chromium-reviews, cbentzel+watch_chromium.org, agl
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Certificate Transparency: Code for unpacking EV cert hashes whitelist The goal is to whitelist logged EV certificates so the requirement of CT for EV certificates can be enabled without waiting for all EV certs to be re-issued. This change adds the code for unpacking the list of (truncated) hashes of EV certificates. The compressed data format is the diff values between the hashes, encoded using Golomb coding. This was suggested by agl as an efficient encoding, since the hash values of the EV certificates are uniformly distributed, so the differences between them are geometrically distributed. See section 4 in: http://algo2.iti.kit.edu/singler/publications/cacheefficientbloomfilters-wea2007.pdf The code that generates the data can be found here: https://github.com/google/certificate-transparency/blob/master/python/utilities/ev_whitelist/golomb_code.py#L27 Currently the code is not hooked into anything, but once the compressed list would be fetched as a component update, we'll start by logging statistics about known vs. unknown EV certs. BUG=339128 Committed: https://crrev.com/743f614e8b1daec08613e6108a6b2b902d5b9b55 Cr-Commit-Position: refs/heads/master@{#293288}

Patch Set 1 #

Patch Set 2 : Linting #

Patch Set 3 : Adding histogram collection #

Patch Set 4 : Rebasing on master #

Total comments: 17

Patch Set 5 : Review comments & linting #

Total comments: 44

Patch Set 6 : #

Total comments: 27

Patch Set 7 : Addressing wtc's comments #

Patch Set 8 : Fixing some compilation issues #

Total comments: 6

Patch Set 9 : Implementing CalculateFingerprint256 on win,mac,ios #

Patch Set 10 : Windows-specific fixes. #

Total comments: 2

Patch Set 11 : Addressing comments + adding method for determining presence of valid EV whitelist. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+559 lines, -4 lines) Patch
A net/cert/ct_ev_whitelist.h View 1 2 3 4 5 6 7 8 9 10 1 chunk +93 lines, -0 lines 0 comments Download
A net/cert/ct_ev_whitelist.cc View 1 2 3 4 5 6 7 8 9 10 1 chunk +158 lines, -0 lines 0 comments Download
A net/cert/ct_ev_whitelist_unittest.cc View 1 2 3 4 5 6 7 8 9 10 1 chunk +179 lines, -0 lines 0 comments Download
M net/cert/x509_certificate.h View 1 chunk +4 lines, -0 lines 0 comments Download
M net/cert/x509_certificate_ios.cc View 1 2 3 4 5 6 7 8 1 chunk +16 lines, -0 lines 0 comments Download
M net/cert/x509_certificate_mac.cc View 1 2 3 4 5 6 7 8 1 chunk +18 lines, -0 lines 0 comments Download
M net/cert/x509_certificate_nss.cc View 1 2 3 4 1 chunk +15 lines, -0 lines 0 comments Download
M net/cert/x509_certificate_openssl.cc View 1 2 3 4 1 chunk +10 lines, -0 lines 0 comments Download
M net/cert/x509_certificate_unittest.cc View 1 2 3 4 5 1 chunk +16 lines, -0 lines 0 comments Download
M net/cert/x509_certificate_win.cc View 1 2 3 4 5 6 7 8 9 1 chunk +21 lines, -0 lines 0 comments Download
M net/net.gypi View 1 2 3 4 5 2 chunks +3 lines, -0 lines 0 comments Download
M net/socket/ssl_client_socket_nss.cc View 1 2 3 4 5 6 7 8 9 10 2 chunks +12 lines, -0 lines 0 comments Download
M tools/metrics/histograms/histograms.xml View 5 chunks +14 lines, -4 lines 0 comments Download

Messages

Total messages: 23 (7 generated)
Eran Messeri
6 years, 5 months ago (2014-08-11 13:22:52 UTC) #1
wtc
Review comments on patch set 4: I will have more time next week. I didn't ...
6 years, 5 months ago (2014-08-14 02:04:44 UTC) #2
Eran Messeri
Addressed all comments, will wait for the more through review next week. I'd like your ...
6 years, 5 months ago (2014-08-14 11:55:08 UTC) #3
wtc
Review comments on patch set 5: I was on vacation yesterday. I reviewed ct_ev_whitelist.* carefully ...
6 years, 5 months ago (2014-08-20 02:52:08 UTC) #4
wtc
Review comments on patch set 5: I reviewed the other files. They are all quite ...
6 years, 5 months ago (2014-08-20 19:04:22 UTC) #5
Eran Messeri
Addressed all comments. Wan-Teh, if you can, please take another look, otherwise agl@ will be ...
6 years, 4 months ago (2014-09-02 12:20:23 UTC) #6
wtc
Patch set 6 LGTM. Please fix the problems I identified. https://codereview.chromium.org/462543002/diff/100001/net/cert/ct_ev_whitelist.cc File net/cert/ct_ev_whitelist.cc (right): https://codereview.chromium.org/462543002/diff/100001/net/cert/ct_ev_whitelist.cc#newcode41 ...
6 years, 4 months ago (2014-09-02 23:03:02 UTC) #7
Eran Messeri
Addressing wtc's comments, adding mpearson for histograms.xml review. https://codereview.chromium.org/462543002/diff/100001/net/cert/ct_ev_whitelist.cc File net/cert/ct_ev_whitelist.cc (right): https://codereview.chromium.org/462543002/diff/100001/net/cert/ct_ev_whitelist.cc#newcode41 net/cert/ct_ev_whitelist.cc:41: } ...
6 years, 4 months ago (2014-09-03 09:38:51 UTC) #9
wtc
Patch set 8 LGTM. I suggest an alternative solution to the initialization of char arrays. ...
6 years, 4 months ago (2014-09-03 14:29:48 UTC) #10
Mark P
https://codereview.chromium.org/462543002/diff/180001/tools/metrics/histograms/histograms.xml File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/462543002/diff/180001/tools/metrics/histograms/histograms.xml#newcode17268 tools/metrics/histograms/histograms.xml:17268: + whitelist Histogram descriptions should clearly state when the ...
6 years, 4 months ago (2014-09-03 17:46:09 UTC) #11
Eran Messeri
Thanks for the quick review, addressed all comments. wtc, please take another look at the ...
6 years, 4 months ago (2014-09-03 20:27:10 UTC) #12
Mark P
histograms.xml lgtm
6 years, 4 months ago (2014-09-03 22:02:29 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/eranm@chromium.org/462543002/200001
6 years, 4 months ago (2014-09-04 10:36:52 UTC) #19
commit-bot: I haz the power
Committed patchset #11 (id:200001) as 65745f920ee45462bf164e2b4cef6e6d891b09a6
6 years, 4 months ago (2014-09-04 11:56:40 UTC) #20
Dmitry Lomov (no reviews)
Reverted in https://chromium.googlesource.com/chromium/src/+/87bf0f714ed33c50a109dc587c6a0e93a7f5d0e3
6 years, 4 months ago (2014-09-04 15:25:02 UTC) #22
commit-bot: I haz the power
6 years, 4 months ago (2014-09-10 03:31:04 UTC) #23
Message was sent while issue was closed.
Patchset 11 (id:??) landed as
https://crrev.com/743f614e8b1daec08613e6108a6b2b902d5b9b55
Cr-Commit-Position: refs/heads/master@{#293288}

Powered by Google App Engine
This is Rietveld 408576698