New issue
Advanced search Search tips

Issue 781275 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

ClientCertResolver: First invocation of CreateSortedCertAndIssuerList is slow

Project Member Reported by pmarko@chromium.org, Nov 3 2017

Issue description

The first invocation of CreateSortedCertAndIssuerList seems to be very slow (~4 seconds on my edgar test machine).

Observations:
- CertLoader gives us ~190 certs. This contains CA certs which have no private key on the machine. Not sure if this is the way it's supposed to be.
- The HasPrivateKey check seems to be slow (calls into NSS).
- The CertLoader::IsCertificateHardwareBacked check seems to be fast (just checks structure members).

Swapping the two conditions seems to reduce the time needed to < 0.1s on my edgar machine.
 
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cf59b09417034d9d5f4cb850d9ac056dbb1635f6

commit cf59b09417034d9d5f4cb850d9ac056dbb1635f6
Author: Pavol Marko <pmarko@chromium.org>
Date: Tue Nov 07 12:43:56 2017

ClientCertResolver: Speed up cert filtering by evaluation order change

When determining which certificates can be used as client certificates,
check if the certificate is hardware-backed first and if it has a
private key second.
Background: The check if a certificate is hardware-backed is less
expensive:
IsCertificateHardwareBacked extracts the slot id (struct member access)
and then calls PK11_IsHW (struct member access).
HasPrivateKey calls PK11_KeyForCertExists, which iterates all tokens
and tries to look up the private key inside each token.

BUG= 781275 

Change-Id: Ie68286e01790cbde305d50227a0e72a0ecacb173
Reviewed-on: https://chromium-review.googlesource.com/753689
Commit-Queue: Pavol Marko <pmarko@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514454}
[modify] https://crrev.com/cf59b09417034d9d5f4cb850d9ac056dbb1635f6/chromeos/network/client_cert_resolver.cc

Labels: OS-Chrome
Status: Fixed (was: Assigned)
Note to self:
The above change landed in in M64 (10110.0.0 / 64.0.3262.0)
https://chromium.googlesource.com/chromium/src/+log/64.0.3261.0..64.0.3262.0?n=10000

It's not necessary to ask the Test team to verify that the above change works properly, as we will know it does if the test result for  bug 781276  is positive.

The work left here would be to make CertLoader give ClientCertResolver only client certificates. This is tracked in bug 781693, so I'm closing this as Fixed.

Comment 4 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Comment 5 by dchan@chromium.org, Jan 23 2018

Status: Fixed (was: Archived)

Sign in to add a comment