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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 11 months ago by wtc
Modified:
1 year, 11 months ago
Reviewers:
Ryan Sleevi, mattm
CC:
chromium-reviews_chromium.org, 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) Lint Patch
M chrome/browser/ssl/ssl_add_cert_handler.cc View 1 chunk +4 lines, -0 lines 0 comments 0 errors Download
M content/browser/renderer_host/x509_user_cert_resource_handler.cc View 1 chunk +15 lines, -2 lines 3 comments 0 errors Download
M net/base/cert_database_nss.cc View 1 2 chunks +14 lines, -0 lines 4 comments 0 errors Download
M net/base/x509_certificate_nss.cc View 1 chunk +2 lines, -0 lines 0 comments 0 errors Download
Trybot results:
Commit:

Messages

Total messages: 4
wtc
mattm: just FYI. rsleevi: this CL is just a proof of concept. It works with ...
1 year, 11 months ago #1
Ryan Sleevi
I wasn't sure if you meant to commit this or just show it, but NACK ...
1 year, 11 months ago #2
wtc
rsleevi: thank you for your comments. I've updated the CL's description and marked it closed ...
1 year, 11 months ago #3
Ryan Sleevi
1 year, 11 months ago #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 1280:2d3e6564b7b6