My favorites | Sign in
Project Home Downloads Wiki Issues Code Search
New issue   Search
for
  Advanced search   Search tips   Subscriptions
Issue 43906: Failed client certificate download (i.e. with <keygen>) does not produce useful debugging information
6 people starred this issue and may be notified of changes. Back to list
Status:  Fixed
Owner:  davidben@chromium.org
Closed:  Sep 2010
Cc:  wtc@chromium.org
M-7

Restricted
  • Only users with EditIssue permission may comment.


Sign in to add a comment
 
Reported by daniel@benoy.name, May 11, 2010
Chrome Version       : 5.0.375.38 (Official Build 46659) beta
Other browsers tested:
Add OK or FAIL after other browsers where you have tested this issue:
     Safari 4: Didn't test
  Firefox 3.x: N/A (Cert works OK)
         IE 7: Didn't test
         IE 8: N/A (Doesn't seem to support my <keygen> page)

What steps will reproduce the problem?
1. Use a site with the <keygen> tag, for example, to generate an (invalid)
client certificate. (although the problem probably occurs even without
using a <keygen> page to generate the cert.  A straight client cert
download will probably have the same problem)
2. If the certificate fails to load, you get the error 'The server returned
an invalid client certificate'
3. The error prompt has no useful information, and I can't find any useful
information anywhere else.

What is the expected result?
Some explanation about why Chrome doesn't like the certificate.

What happens instead?
As far as I can tell, the certs I'm testing with are valid (and they work
with firefox), so unless Chrome is willing to share what, precisely, its
problem is, I'm unable to fix it.  That's a problem.
Jun 6, 2010
#1 ryan.sle...@gmail.com
Is it possible you're encountering bug #37142 , which is where intermediate 
certificates are not (yet) supported?
Jun 7, 2010
#2 daniel@benoy.name
Nope.  My particular problem didn't send back (or even involve) intermediate CAs.

However, I submitted this bug about the inadequate debugging information, and that
problem is reproducible every time it fails, whatever the reason.
Jun 7, 2010
#3 ryan.sle...@gmail.com
For what it's worth, Firefox appears to use only a single error message as well when checking the validity of 
the returned certificate(s), with a single additional message used to inform of chain validity if any 
additional certificates returned. No useful information is in the prompt, and no use information is included 
anywhere else, AFAICT. I haven't checked Opera yet, but I'm fairly certain from earlier tests it's the same. 

Though I'm perfectly aware that the issue is for requesting more diagnostics, I feel it at least appropriate to 
address the root issue that caused the request. In it's current implementation, only a single DER certificate 
can be returned. Bug #37142 is tracking support for both intermediates and for other formats (eg: PEM). Given 
that it's importing fine into Firefox, is it possible you're returning the single certificate in PEM form, 
something not supported on Windows or Mac presently?
Jun 7, 2010
#4 daniel@benoy.name
I'm not really interested in getting my particular implementation to work with chrome
anymore.  We've decided to just drop chrome support and require firefox.
Jun 17, 2010
#5 wtc@chromium.org
daniel: thank you very much for the bug report.  You made the right call to
not support Chrome right now because Chrome's <keygen> is not ready for
prime time even though it works in some cases.  If you aren't interested in
helping us track down why Chrome cannot import your certs, we'll restrict the
scope of this bug report to the error reporting issue.  Like Ryan Sleevi,
I am interested in tracking down the underlying cause of the "invalid client
certificate" error.  The following describes all possible reasons for the
failure.

davidben: good error reporting is important when we support customers.
The error message "The server returned an invalid client certificate"
is IDS_ADD_CERT_ERR_INVALID_CERT.  It is only used in
src/chrome/browser/ssl/ssl_add_cert_handler.cc:

void SSLAddCertHandler::RunUI() {
  int cert_error;
  {
    net::CertDatabase db;
    cert_error = db.CheckUserCert(cert_);
  }
  if (cert_error != net::OK) {
    // TODO(snej): Map cert_error to a more specific error message.
    ShowError(l10n_util::GetStringUTF16(IDS_ADD_CERT_ERR_INVALID_CERT));
    Finished(false);
    return;
  }
  AskToAddCert();
}

A first step is to include |cert_error| in the error message.  There
are three possible values of |cert_error|:

int CertDatabase::CheckUserCert(X509Certificate* cert) {
  if (!cert)
    return ERR_CERT_INVALID;
  if (cert->HasExpired())
    return ERR_CERT_DATE_INVALID;

  std::string encoded_info = GetSubjectPublicKeyInfo(cert);
  KeygenHandler::Cache* cache = KeygenHandler::Cache::GetInstance();
  KeygenHandler::KeyLocation location;

  if (encoded_info.empty() || !cache->Find(encoded_info, &location) ||
      !LinkCertToPrivateKey(cert, location))
    return ERR_NO_PRIVATE_KEY_FOR_CERT;

  return OK;
}

|cert_| is NULL if X509Certificate::CreateFromBytes() fails,
most likely a decoding error:

bool X509UserCertResourceHandler::OnResponseCompleted(
    int request_id,
    const URLRequestStatus& urs,
    const std::string& sec_info) {
  if (urs.status() != URLRequestStatus::SUCCESS)
    return false;

  // TODO(gauravsh): Verify that 'request_id' was actually a keygen form post
  // and only then import the certificate.
  AssembleResource();
  scoped_refptr<net::X509Certificate> cert =
      net::X509Certificate::CreateFromBytes(resource_buffer_->data(),
                                            content_length_);
  // The handler will run the UI and delete itself when it's finished.
  new SSLAddCertHandler(request_, cert);
  return true;
}

We can consider using a better error code than ERR_CERT_INVALID
for this error condition, or add error logging code to
X509Certificate::CreateFromBytes.

Finally, we can consider creating custom error messages for these
three errors.
Status: Assigned
Owner: david...@chromium.org
Cc: w...@chromium.org
Labels: -Area-Undefined Area-Internals Internals-Network Mstone-7
Jul 15, 2010
#6 bugdroid1@gmail.com
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=52499 

------------------------------------------------------------------------
r52499 | davidben@chromium.org | 2010-07-15 10:52:36 -0700 (Thu, 15 Jul 2010) | 8 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/app/generated_resources.grd?r1=52499&r2=52498
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ssl/ssl_add_cert_handler.cc?r1=52499&r2=52498

Display the error code when certificates fail to add

We really want to make error strings, but this is a start.

BUG=43906
TEST=none

Review URL: http://codereview.chromium.org/2859026
------------------------------------------------------------------------

Sep 3, 2010
#7 davidben@chromium.org
Moving this to Mstone-X. If we end up making localized error strings, it probably won't in M7, seeing as (I believe) the string freeze has passed?
Labels: -Mstone-7 Mstone-X
Sep 7, 2010
#8 wtc@chromium.org
Marked the bug fixed in M7.

Displaying the error code and its symbolic name is good enough.
To summarize, the current version may return three possible error
codes:
  ERR_CERT_INVALID
  ERR_CERT_DATE_INVALID
  ERR_NO_PRIVATE_KEY_FOR_CERT

Since we have limited time, I'd prefer adding new error codes
than creating custom error messages.  For example, I noted in
comment 5 that a new error code could replace ERR_CERT_INVALID
here:

  |cert_| is NULL if X509Certificate::CreateFromBytes() fails,
  most likely a decoding error:

  bool X509UserCertResourceHandler::OnResponseCompleted(
      int request_id,
      const URLRequestStatus& urs,
      const std::string& sec_info) {
    if (urs.status() != URLRequestStatus::SUCCESS)
      return false;

    // TODO(gauravsh): Verify that 'request_id' was actually a keygen form post
    // and only then import the certificate.
    AssembleResource();
    scoped_refptr<net::X509Certificate> cert =
        net::X509Certificate::CreateFromBytes(resource_buffer_->data(),
                                              content_length_);
    // The handler will run the UI and delete itself when it's finished.
    new SSLAddCertHandler(request_, cert, render_process_host_id_,
                          render_view_id_);
    return true;
  }

  We can consider using a better error code than ERR_CERT_INVALID
  for this error condition, or add error logging code to
  X509Certificate::CreateFromBytes.

Please open a new bug report if you want to work on this.
Status: Fixed
Labels: -Mstone-X Mstone-7
Oct 12, 2012
#9 bugdro...@chromium.org
This issue has been closed for some time. No one will pay attention to new comments.
If you are seeing this bug or have new data, please click New Issue to start a new bug.
Labels: Restrict-AddIssueComment-Commit
Mar 10, 2013
#10 bugdro...@chromium.org
(No comment was entered for this change.)
Labels: -Area-Internals -Internals-Network -Mstone-7 M-7 Cr-Internals Cr-Internals-Network
Mar 13, 2013
#11 bugdro...@chromium.org
(No comment was entered for this change.)
Labels: -Restrict-AddIssueComment-Commit Restrict-AddIssueComment-EditIssue
Sign in to add a comment

Powered by Google Project Hosting