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

Issue 799916 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

MultiLogCTVerifier in IOThread has empty CT log list

Project Member Reported by robpercival@chromium.org, Jan 8 2018

Issue description

Chrome Version: 

The MultiLogCTVerifier in IOThread (https://cs.chromium.org/chromium/src/chrome/browser/io_thread.cc?l=795&rcl=30cf78450be041e3329a4d0f8c8a485724564b84) is currently being passed an empty list of CT logs, due to a change in the order of initialization introduced by https://crrev.com/c/768790. Instead, it should be passed the set of known CT logs. 

Some requests are unaffected (e.g. those performed by the integration test in https://crrev.com/c/806536), because they use the MultiLogCTVerifier setup in ProfileIOData (https://cs.chromium.org/chromium/src/chrome/browser/profiles/profile_io_data.cc?l=1182&rcl=30cf78450be041e3329a4d0f8c8a485724564b84), which gets the correct list of CT logs. The integration test should be improved to also test the MultiLogCTVerifier in IOThread.
 
Oh wow! Thanks for catching this - and for adding tests to prevent future regressions!
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 8 2018

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

commit 51cfa60b9d0c0dd511982774c896ce3860b08e8a
Author: Rob Percival <robpercival@chromium.org>
Date: Mon Jan 08 17:04:36 2018

Populate globals_->ct_logs before calling ConstructSystemRequestContext()

globals_->ct_logs is passed to a net::MultiLogCTVerifier within
ConstructSystemRequestContext().

Bug:  799916 
Change-Id: I457d0a6ca4b6daea7626daacbd544a782c122154
Reviewed-on: https://chromium-review.googlesource.com/853956
Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: Rob Percival <robpercival@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527651}
[modify] https://crrev.com/51cfa60b9d0c0dd511982774c896ce3860b08e8a/chrome/browser/io_thread.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Mar 28 2018

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

commit 5c442f87659b835707a5bdc9ed2e68915186d26e
Author: Rob Percival <robpercival@chromium.org>
Date: Wed Mar 28 22:10:57 2018

Integration test for CT

Checks that histograms are correctly populated after navigating to a
website that supplies SCTs complying with Chrome's CT policy.

Bug: 506227,  799916 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: I0d76da901b438b2b28a9ef4ccb3decf347a5f537
Reviewed-on: https://chromium-review.googlesource.com/806536
Commit-Queue: Rob Percival <robpercival@chromium.org>
Reviewed-by: Ryan Sleevi <rsleevi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546616}
[modify] https://crrev.com/5c442f87659b835707a5bdc9ed2e68915186d26e/chrome/browser/io_thread.cc
[modify] https://crrev.com/5c442f87659b835707a5bdc9ed2e68915186d26e/chrome/browser/io_thread.h
[modify] https://crrev.com/5c442f87659b835707a5bdc9ed2e68915186d26e/chrome/browser/ssl/DEPS
[modify] https://crrev.com/5c442f87659b835707a5bdc9ed2e68915186d26e/chrome/browser/ssl/cert_verifier_browser_test.cc
[add] https://crrev.com/5c442f87659b835707a5bdc9ed2e68915186d26e/chrome/browser/ssl/certificate_transparency_browsertest.cc
[modify] https://crrev.com/5c442f87659b835707a5bdc9ed2e68915186d26e/chrome/test/BUILD.gn
[modify] https://crrev.com/5c442f87659b835707a5bdc9ed2e68915186d26e/components/certificate_transparency/BUILD.gn
[add] https://crrev.com/5c442f87659b835707a5bdc9ed2e68915186d26e/components/certificate_transparency/features.cc
[add] https://crrev.com/5c442f87659b835707a5bdc9ed2e68915186d26e/components/certificate_transparency/features.h
[modify] https://crrev.com/5c442f87659b835707a5bdc9ed2e68915186d26e/components/certificate_transparency/single_tree_tracker.cc
[modify] https://crrev.com/5c442f87659b835707a5bdc9ed2e68915186d26e/components/certificate_transparency/single_tree_tracker.h
[modify] https://crrev.com/5c442f87659b835707a5bdc9ed2e68915186d26e/components/certificate_transparency/tree_state_tracker.cc
[modify] https://crrev.com/5c442f87659b835707a5bdc9ed2e68915186d26e/components/certificate_transparency/tree_state_tracker_unittest.cc
[modify] https://crrev.com/5c442f87659b835707a5bdc9ed2e68915186d26e/net/BUILD.gn
[modify] https://crrev.com/5c442f87659b835707a5bdc9ed2e68915186d26e/net/data/ssl/certificates/README
[add] https://crrev.com/5c442f87659b835707a5bdc9ed2e68915186d26e/net/data/ssl/certificates/comodo-chain.pem
[modify] https://crrev.com/5c442f87659b835707a5bdc9ed2e68915186d26e/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter

Status: Fixed (was: Started)
Thanks go to Michal Pichlinski from Opera who tipped me off about this bug. We've now got an integration test that will catch such regressions in the future.

Sign in to add a comment