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

Issue 6793041: net: add ability to distinguish user-added root CAs. (Closed)

Created:
9 years, 8 months ago by agl
Modified:
9 years, 7 months ago
Reviewers:
wtc
CC:
chromium-reviews, pam+watch_chromium.org, cbentzel+watch_chromium.org, darin-cc_chromium.org, PaweĊ‚ Hajdan Jr.
Visibility:
Public.

Description

net: add ability to distinguish user-added root CAs. We have several places where a need to distinguish `real' root CAs from user-added root CAs will be useful: 1) Monoscope wants to inspect correctly signed, but unknown certificates, but doesn't want to deal with proxy MITM certificates. 2) HSTS is likely to add a method for pinning to a certificate, but we don't want to break every proxy MITM with it. This change adds several lists of known, `real' roots. These lists present an ongoing maintainance issue. However, in the event that the lists are incomplete in the future, we fail open. This is because roots not in these lists are treated as user-added and user-added roots have more authority than `real' roots. In some sense, this is a problem because it might be a security issue that new roots are given too much authority. On the other hand, we're not breaking things when we're behind on updating the lists so the maintainance issue isn't too pressing. BUG=none TEST=none Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=80778

Patch Set 1 #

Patch Set 2 : ... #

Total comments: 30

Patch Set 3 : ... #

Patch Set 4 : ... #

Patch Set 5 : ... #

Total comments: 16
Unified diffs Side-by-side diffs Delta from patch set Stats (+1104 lines, -4 lines) Patch
M net/base/cert_verify_result.h View 1 2 3 chunks +8 lines, -1 line 2 comments Download
M net/base/x509_certificate.h View 1 2 3 4 3 chunks +11 lines, -1 line 2 comments Download
M net/base/x509_certificate.cc View 1 2 3 4 chunks +19 lines, -0 lines 0 comments Download
M net/base/x509_certificate_mac.cc View 1 2 3 4 3 chunks +18 lines, -1 line 3 comments Download
A net/base/x509_certificate_mac_known_roots.h View 1 2 3 4 1 chunk +342 lines, -0 lines 0 comments Download
M net/base/x509_certificate_nss.cc View 1 2 3 3 chunks +19 lines, -1 line 4 comments Download
M net/base/x509_certificate_unittest.cc View 1 2 1 chunk +13 lines, -0 lines 3 comments Download
M net/base/x509_certificate_win.cc View 1 2 3 4 4 chunks +19 lines, -0 lines 2 comments Download
A net/base/x509_certificate_win_known_roots.h View 1 2 3 4 1 chunk +655 lines, -0 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
agl
Note: the try server are failing on this CL because they can't add nist.der (a ...
9 years, 8 months ago (2011-04-05 13:18:38 UTC) #1
wtc
Most of my comments are style nits. Two are marked with "BUG". One suggestion on ...
9 years, 8 months ago (2011-04-06 04:28:38 UTC) #2
agl
(I'm still working the patch past the try bots.) http://codereview.chromium.org/6793041/diff/2001/net/base/cert_verify_result.h File net/base/cert_verify_result.h (right): http://codereview.chromium.org/6793041/diff/2001/net/base/cert_verify_result.h#newcode24 net/base/cert_verify_result.h:24: ...
9 years, 8 months ago (2011-04-06 19:02:02 UTC) #3
wtc
LGTM. Note the two comments marked with "BUG" below. http://codereview.chromium.org/6793041/diff/6004/net/base/cert_verify_result.h File net/base/cert_verify_result.h (right): http://codereview.chromium.org/6793041/diff/6004/net/base/cert_verify_result.h#newcode37 net/base/cert_verify_result.h:37: ...
9 years, 8 months ago (2011-04-07 05:01:54 UTC) #4
agl
http://codereview.chromium.org/6793041/diff/6004/net/base/cert_verify_result.h File net/base/cert_verify_result.h (right): http://codereview.chromium.org/6793041/diff/6004/net/base/cert_verify_result.h#newcode37 net/base/cert_verify_result.h:37: // is_issued_by_known_root is true if recognise the root CA ...
9 years, 8 months ago (2011-04-07 15:02:48 UTC) #5
wtc
9 years, 8 months ago (2011-04-08 00:39:17 UTC) #6
http://codereview.chromium.org/6793041/diff/6004/net/base/x509_certificate_un...
File net/base/x509_certificate_unittest.cc (right):

http://codereview.chromium.org/6793041/diff/6004/net/base/x509_certificate_un...
net/base/x509_certificate_unittest.cc:494:
EXPECT_TRUE(verify_result.is_issued_by_known_root);
On 2011/04/07 15:02:49, agl wrote:
> Now, expiry is a problem
> here but I can't find a way around it :( The test will blow up in the future.

Yes.  The X509CertificateTest unit tests that use real
certificates all have this problem.  We should at least
document when the certificate will expire so that build
sheriffs will know they can disable the test safely when
the test blows up.

Powered by Google App Engine
This is Rietveld 408576698