Chromium Code Reviews
Help | Chromium Project | Sign in
(8)

Issue 10160007: Parse an application/x-x509-user-cert response with (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 years, 1 month ago by wtc
Modified:
3 years ago
Reviewers:
Ryan Sleevi, mattm
CC:
chromium-reviews, joi+watch-content_chromium.org, darin-cc_chromium.org, cbentzel+watch_chromium.org, jam
Visibility:
Public.

Description

[This CL is for reference only.] Parse an application/x-x509-user-cert response with net::X509Certificate::CreateCertificateListFromBytes(), which supports three formats (in particular PKCS #7). Import the intermediate CA certificates in the response (only implemented for NSS). R=rsleevi@chromium.org,mattm@chromium.org BUG=37142 TEST=none

Patch Set 1 #

Patch Set 2 : Add back a blank line deleted by accident #

Total comments: 7
Unified diffs Side-by-side diffs Delta from patch set Stats (+35 lines, -2 lines) Patch
M chrome/browser/ssl/ssl_add_cert_handler.cc View 1 chunk +4 lines, -0 lines 0 comments Download
M content/browser/renderer_host/x509_user_cert_resource_handler.cc View 1 chunk +15 lines, -2 lines 3 comments Download
M net/base/cert_database_nss.cc View 1 2 chunks +14 lines, -0 lines 4 comments Download
M net/base/x509_certificate_nss.cc View 1 chunk +2 lines, -0 lines 0 comments Download
Trybot results:
Commit: CQ not working?

Messages

Total messages: 4 (0 generated)
wtc
mattm: just FYI. rsleevi: this CL is just a proof of concept. It works with ...
3 years, 1 month ago (2012-04-26 23:11:53 UTC) #1
Ryan Sleevi
I wasn't sure if you meant to commit this or just show it, but NACK ...
3 years, 1 month ago (2012-04-27 00:55:48 UTC) #2
wtc
rsleevi: thank you for your comments. I've updated the CL's description and marked it closed ...
3 years ago (2012-04-27 21:16:49 UTC) #3
Ryan Sleevi
3 years ago (2012-04-27 21:24:21 UTC) #4
https://chromiumcodereview.appspot.com/10160007/diff/6001/content/browser/ren...
File content/browser/renderer_host/x509_user_cert_resource_handler.cc (right):

https://chromiumcodereview.appspot.com/10160007/diff/6001/content/browser/ren...
content/browser/renderer_host/x509_user_cert_resource_handler.cc:115:
intermediate_certs);
On 2012/04/27 21:16:50, wtc wrote:
> 
> On 2012/04/27 00:55:48, Ryan Sleevi wrote:
> > note: This is not an accurate interpretation of the results, from what I've
> > seen.
> 
> You mean we cannot assume the first certificate in the
> certificate list is the leaf user certificate?  I wasn't
> sure if we could depend on this assumption either; it just
> happened to be true for COMODO's PKCS #7 response.

Correct. The behaviour of Firefox is to do a first pass scan to see if it
notices issuing order and, if so, reverse if appropriate.

It then checks the first cert for private key possession before continuing.

It then imports the cert (regardless of any secondary trust / issued by known CA
bits)

Then, for each remaining cert in the range [1..i..n], it tries to verify that
cert [i], supplying [i+1..n] as optional intermediates, chains to a
known/trusted root.

https://chromiumcodereview.appspot.com/10160007/diff/6001/net/base/cert_datab...
File net/base/cert_database_nss.cc (right):

https://chromiumcodereview.appspot.com/10160007/diff/6001/net/base/cert_datab...
net/base/cert_database_nss.cc:89: PK11_ImportCert(slot, intermediate_cert,
CK_INVALID_HANDLE, nickname,
On 2012/04/27 21:16:50, wtc wrote:
> On 2012/04/27 00:55:48, Ryan Sleevi wrote:
> >
> > This is why Firefox ensures a chain can be built for each certificate,
before
> > importing.
> 
> We can copy Firefox's behavior.  This means if client certs
> are issued by a private CA that is not in the browser's
> trusted root CA list, that private CA has to issue the client
> certs directly and can't issue them from an intermediate CA
> (because the browser won't import the intermediate CA cert).

What Firefox does is described in my previous comment. It effectively
accomplishes the same thing, in that no intermediates will be imported that do
not chain to a (previously trusted/imported/built-in) root.

That's were my comment about UI behaviour comes in, since there are typically
one or two (such as non-cross-signed root certs) in these bundles.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld ec887be