New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 612655 link

Starred by 1 user
Status: Verified
Owner:
rsleevi@chromium.org
Closed: Jun 2016
Cc:
█ rtenneti@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug



Sign in to add a comment

Cleanup: Decouple caching from the MultiThreadedCertVerifier

Project Member Reported by rsleevi@chromium.org, May 18 2016 
It's surprising and unexpected that MultiThreadedCertVerifier is also responsible for managing the cache of certificate verification results.

To make it easier to experiment with different caching strategies, including cache persistence, split the cache management out of MultiThreadedCertVerifier, so that the MultiThreadedCertVerifier is only responsible for handling the threading/management aspect, and a CachingCertVerifier can impose memoizaton upon it.
Project Member

Comment 1 by bugdroid1@chromium.org, May 20 2016 

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

commit e51d72bacc26ac549ee7de34b6a96916e89cc5a9
Author: rsleevi <rsleevi@chromium.org>
Date: Fri May 20 02:49:08 2016

Introduce CertVerifier::RequestParams

In anticipation of divorcing caching from multi-threading,
move the MultiThreadedCertVerifier's RequestParams up to
CertVerifier, as part of a public, stable interface for
handling verification request. This mirrors the notion of
HostResolver's RequestInfo.

While Verify() does not yet take a RequestParams (which
will come later), this provides a common interface to use
as a key for determining if two requests should be treated
the same (e.g. for purposes of caching or joining existing
requests).

BUG= 612655 

Review-Url: https://codereview.chromium.org/1987113002
Cr-Commit-Position: refs/heads/master@{#394967}

[modify] https://crrev.com/e51d72bacc26ac549ee7de34b6a96916e89cc5a9/net/cert/cert_verifier.cc
[modify] https://crrev.com/e51d72bacc26ac549ee7de34b6a96916e89cc5a9/net/cert/cert_verifier.h
[add] https://crrev.com/e51d72bacc26ac549ee7de34b6a96916e89cc5a9/net/cert/cert_verifier_unittest.cc
[modify] https://crrev.com/e51d72bacc26ac549ee7de34b6a96916e89cc5a9/net/cert/multi_threaded_cert_verifier.cc
[modify] https://crrev.com/e51d72bacc26ac549ee7de34b6a96916e89cc5a9/net/cert/multi_threaded_cert_verifier.h
[modify] https://crrev.com/e51d72bacc26ac549ee7de34b6a96916e89cc5a9/net/cert/multi_threaded_cert_verifier_unittest.cc
[modify] https://crrev.com/e51d72bacc26ac549ee7de34b6a96916e89cc5a9/net/net.gypi
Project Member

Comment 2 by bugdroid1@chromium.org, Jun 8 2016 

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

commit 06bd78559d65ac5b38b7bcfd370baf35532e79ea
Author: rsleevi <rsleevi@chromium.org>
Date: Wed Jun 08 22:34:46 2016

Update CertVerifier::Verify to use RequestParams instead

CertVerifier::Verify() has some inconsistencies with
respect to how it handles additional trust anchors, which
complicates the layering and the caching.

Rather than just add an additional parameter to Verify(),
convert everything to use RequestParams, which also defines how
the caching/hashing is done.

BUG= 612655 
TBR=brettw

Review-Url: https://codereview.chromium.org/1994353002
Cr-Commit-Position: refs/heads/master@{#398707}

[modify] https://crrev.com/06bd78559d65ac5b38b7bcfd370baf35532e79ea/blimp/net/exact_match_cert_verifier.cc
[modify] https://crrev.com/06bd78559d65ac5b38b7bcfd370baf35532e79ea/blimp/net/exact_match_cert_verifier.h
[modify] https://crrev.com/06bd78559d65ac5b38b7bcfd370baf35532e79ea/chrome/browser/chromeos/policy/policy_cert_verifier.cc
[modify] https://crrev.com/06bd78559d65ac5b38b7bcfd370baf35532e79ea/chrome/browser/chromeos/policy/policy_cert_verifier.h
[modify] https://crrev.com/06bd78559d65ac5b38b7bcfd370baf35532e79ea/chrome/browser/chromeos/policy/policy_cert_verifier_browsertest.cc
[modify] https://crrev.com/06bd78559d65ac5b38b7bcfd370baf35532e79ea/chrome/browser/extensions/api/gcd_private/privet_v3_context_getter.cc
[modify] https://crrev.com/06bd78559d65ac5b38b7bcfd370baf35532e79ea/chrome/browser/extensions/api/platform_keys/verify_trust_api.cc
[modify] https://crrev.com/06bd78559d65ac5b38b7bcfd370baf35532e79ea/extensions/browser/api/cast_channel/cast_socket.cc
[modify] https://crrev.com/06bd78559d65ac5b38b7bcfd370baf35532e79ea/google_apis/gcm/tools/mcs_probe.cc
[modify] https://crrev.com/06bd78559d65ac5b38b7bcfd370baf35532e79ea/ios/web/net/cert_verifier_block_adapter.cc
[modify] https://crrev.com/06bd78559d65ac5b38b7bcfd370baf35532e79ea/net/cert/cert_verifier.cc
[modify] https://crrev.com/06bd78559d65ac5b38b7bcfd370baf35532e79ea/net/cert/cert_verifier.h
[modify] https://crrev.com/06bd78559d65ac5b38b7bcfd370baf35532e79ea/net/cert/cert_verifier_unittest.cc
[modify] https://crrev.com/06bd78559d65ac5b38b7bcfd370baf35532e79ea/net/cert/mock_cert_verifier.cc
[modify] https://crrev.com/06bd78559d65ac5b38b7bcfd370baf35532e79ea/net/cert/mock_cert_verifier.h
[modify] https://crrev.com/06bd78559d65ac5b38b7bcfd370baf35532e79ea/net/cert/multi_threaded_cert_verifier.cc
[modify] https://crrev.com/06bd78559d65ac5b38b7bcfd370baf35532e79ea/net/cert/multi_threaded_cert_verifier.h
[modify] https://crrev.com/06bd78559d65ac5b38b7bcfd370baf35532e79ea/net/cert/multi_threaded_cert_verifier_unittest.cc
[modify] https://crrev.com/06bd78559d65ac5b38b7bcfd370baf35532e79ea/net/cert_net/nss_ocsp_unittest.cc
[modify] https://crrev.com/06bd78559d65ac5b38b7bcfd370baf35532e79ea/net/quic/crypto/proof_verifier_chromium.cc
[modify] https://crrev.com/06bd78559d65ac5b38b7bcfd370baf35532e79ea/net/quic/crypto/proof_verifier_chromium_test.cc
[modify] https://crrev.com/06bd78559d65ac5b38b7bcfd370baf35532e79ea/net/socket/ssl_client_socket_impl.cc
[modify] https://crrev.com/06bd78559d65ac5b38b7bcfd370baf35532e79ea/net/tools/quic/quic_client_bin.cc
[modify] https://crrev.com/06bd78559d65ac5b38b7bcfd370baf35532e79ea/net/tools/quic/quic_simple_client_bin.cc
[modify] https://crrev.com/06bd78559d65ac5b38b7bcfd370baf35532e79ea/remoting/protocol/ssl_hmac_channel_authenticator.cc
Project Member

Comment 3 by bugdroid1@chromium.org, Jun 13 2016 

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

commit 6df5418b76ec5d9daa9df35e6f616129b72a1a5d
Author: rsleevi <rsleevi@chromium.org>
Date: Mon Jun 13 14:34:23 2016

Move caching out of MultiThreadedCertVerifier

Split out the logic for caching cert verification from the
MultiThreadedCertVerifier, and into the CachingCertVerifier,
which can add caching/memoization to any CertVerifier.

BUG= 612655 

Review-Url: https://codereview.chromium.org/1991653002
Cr-Commit-Position: refs/heads/master@{#399446}

[modify] https://crrev.com/6df5418b76ec5d9daa9df35e6f616129b72a1a5d/chrome/browser/chromeos/policy/policy_cert_verifier.cc
[modify] https://crrev.com/6df5418b76ec5d9daa9df35e6f616129b72a1a5d/chrome/browser/io_thread.cc
[modify] https://crrev.com/6df5418b76ec5d9daa9df35e6f616129b72a1a5d/chrome/browser/profiles/profile_io_data.cc
[modify] https://crrev.com/6df5418b76ec5d9daa9df35e6f616129b72a1a5d/ios/chrome/browser/ios_chrome_io_thread.mm
[add] https://crrev.com/6df5418b76ec5d9daa9df35e6f616129b72a1a5d/net/cert/caching_cert_verifier.cc
[add] https://crrev.com/6df5418b76ec5d9daa9df35e6f616129b72a1a5d/net/cert/caching_cert_verifier.h
[add] https://crrev.com/6df5418b76ec5d9daa9df35e6f616129b72a1a5d/net/cert/caching_cert_verifier_unittest.cc
[modify] https://crrev.com/6df5418b76ec5d9daa9df35e6f616129b72a1a5d/net/cert/cert_verifier.cc
[modify] https://crrev.com/6df5418b76ec5d9daa9df35e6f616129b72a1a5d/net/cert/multi_threaded_cert_verifier.cc
[modify] https://crrev.com/6df5418b76ec5d9daa9df35e6f616129b72a1a5d/net/cert/multi_threaded_cert_verifier.h
[modify] https://crrev.com/6df5418b76ec5d9daa9df35e6f616129b72a1a5d/net/cert/multi_threaded_cert_verifier_unittest.cc
[modify] https://crrev.com/6df5418b76ec5d9daa9df35e6f616129b72a1a5d/net/net.gypi
Project Member

Comment 4 by bugdroid1@chromium.org, Jun 15 2016 

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

commit 6df5418b76ec5d9daa9df35e6f616129b72a1a5d
Author: rsleevi <rsleevi@chromium.org>
Date: Mon Jun 13 14:34:23 2016

Move caching out of MultiThreadedCertVerifier

Split out the logic for caching cert verification from the
MultiThreadedCertVerifier, and into the CachingCertVerifier,
which can add caching/memoization to any CertVerifier.

BUG= 612655 

Review-Url: https://codereview.chromium.org/1991653002
Cr-Commit-Position: refs/heads/master@{#399446}

[modify] https://crrev.com/6df5418b76ec5d9daa9df35e6f616129b72a1a5d/chrome/browser/chromeos/policy/policy_cert_verifier.cc
[modify] https://crrev.com/6df5418b76ec5d9daa9df35e6f616129b72a1a5d/chrome/browser/io_thread.cc
[modify] https://crrev.com/6df5418b76ec5d9daa9df35e6f616129b72a1a5d/chrome/browser/profiles/profile_io_data.cc
[modify] https://crrev.com/6df5418b76ec5d9daa9df35e6f616129b72a1a5d/ios/chrome/browser/ios_chrome_io_thread.mm
[add] https://crrev.com/6df5418b76ec5d9daa9df35e6f616129b72a1a5d/net/cert/caching_cert_verifier.cc
[add] https://crrev.com/6df5418b76ec5d9daa9df35e6f616129b72a1a5d/net/cert/caching_cert_verifier.h
[add] https://crrev.com/6df5418b76ec5d9daa9df35e6f616129b72a1a5d/net/cert/caching_cert_verifier_unittest.cc
[modify] https://crrev.com/6df5418b76ec5d9daa9df35e6f616129b72a1a5d/net/cert/cert_verifier.cc
[modify] https://crrev.com/6df5418b76ec5d9daa9df35e6f616129b72a1a5d/net/cert/multi_threaded_cert_verifier.cc
[modify] https://crrev.com/6df5418b76ec5d9daa9df35e6f616129b72a1a5d/net/cert/multi_threaded_cert_verifier.h
[modify] https://crrev.com/6df5418b76ec5d9daa9df35e6f616129b72a1a5d/net/cert/multi_threaded_cert_verifier_unittest.cc
[modify] https://crrev.com/6df5418b76ec5d9daa9df35e6f616129b72a1a5d/net/net.gypi

Comment 5 by eroman@chromium.org, Jun 21 2016

Components: -Internals>Network>SSL Internals>Network>Certificate

Comment 6 by rsleevi@chromium.org, Jun 21 2016

Status: Verified (was: Started)
►

Sign in to add a comment