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

Issue 847933 link

Starred by 1 user
Status: Assigned
Owner:
█ ericrobinson@chromium.org
Last visit > 30 days ago
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Augment RulesetService to store the ruleset checksum in prefs

Project Member Reported by csharrison@chromium.org, May 30 2018 
Data corruption on-disk could cause the indexed ruleset to behave unpredictably, or even lead to crashes.

There have been previous discussions about storing the checksum directly in the flatbuffer, but it would be simplest to store it in subresource_filter::IndexedRulesetVersion, alongside the content / format version.

This way, our ruleset verification step can also ensure the contents did not change on disk by comparing the checksum to the one in prefs.

Comment 1 by ericrobinson@chromium.org, May 30 2018

Owner: ericrobinson@chromium.org
I'll take a look.

Comment 2 by csharrison@chromium.org, May 30 2018

Status: Assigned (was: Available)
Keep in mind that there may need to be a migration strategy here, since the existing version stored in prefs does _not_ have the checksum. I think the easiest solution would be, at ruleset verification time, to just write the checksum of the existing file to the pref if the checksum doesn't exist (i.e. assume the current on-disk file is OK).

We can probably remove this migration path once the indexed ruleset bumps a format version (in which case we have to re-index and re-write the ruleset, ensuring current rulesets have the pref properly written).
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 23 

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

commit 20fa771e874c7e368299ec7cba6776a810d40aaf
Author: Eric Robinson <ericrobinson@chromium.org>
Date: Thu Aug 23 18:12:32 2018

Adding checksum for ruleset file.

This adds a checksum that's computed when a ruleset is indexed for the first time.
The checksum is stored in prefs and used to determine if the ruleset file is
valid when loaded.  This prevents crashes from errors not picked up by the
flatbuffer verifier when the ruleset file has unexpectedly changed on disk.

Bug: 847933
Change-Id: Idc4cdd8520b2b53d27e66d98184d58767891db31
Reviewed-on: https://chromium-review.googlesource.com/1093152
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Commit-Queue: Eric Robinson <ericrobinson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585545}
[modify] https://crrev.com/20fa771e874c7e368299ec7cba6776a810d40aaf/chrome/browser/browser_process_impl.cc
[modify] https://crrev.com/20fa771e874c7e368299ec7cba6776a810d40aaf/chrome/browser/component_updater/subresource_filter_component_installer_unittest.cc
[add] https://crrev.com/20fa771e874c7e368299ec7cba6776a810d40aaf/chrome/browser/subresource_filter/ruleset_browsertest.cc
[modify] https://crrev.com/20fa771e874c7e368299ec7cba6776a810d40aaf/chrome/browser/subresource_filter/subresource_filter_browsertest.cc
[modify] https://crrev.com/20fa771e874c7e368299ec7cba6776a810d40aaf/chrome/browser/subresource_filter/subresource_filter_test_harness.cc
[modify] https://crrev.com/20fa771e874c7e368299ec7cba6776a810d40aaf/chrome/test/BUILD.gn
[modify] https://crrev.com/20fa771e874c7e368299ec7cba6776a810d40aaf/components/subresource_filter/content/browser/activation_state_computing_navigation_throttle_unittest.cc
[modify] https://crrev.com/20fa771e874c7e368299ec7cba6776a810d40aaf/components/subresource_filter/content/browser/async_document_subresource_filter_unittest.cc
[modify] https://crrev.com/20fa771e874c7e368299ec7cba6776a810d40aaf/components/subresource_filter/content/browser/content_ruleset_service.cc
[modify] https://crrev.com/20fa771e874c7e368299ec7cba6776a810d40aaf/components/subresource_filter/content/browser/content_ruleset_service.h
[modify] https://crrev.com/20fa771e874c7e368299ec7cba6776a810d40aaf/components/subresource_filter/content/browser/content_ruleset_service_unittest.cc
[modify] https://crrev.com/20fa771e874c7e368299ec7cba6776a810d40aaf/components/subresource_filter/content/browser/content_subresource_filter_throttle_manager_unittest.cc
[modify] https://crrev.com/20fa771e874c7e368299ec7cba6776a810d40aaf/components/subresource_filter/content/browser/subframe_navigation_filtering_throttle_unittest.cc
[modify] https://crrev.com/20fa771e874c7e368299ec7cba6776a810d40aaf/components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle_unittest.cc
[modify] https://crrev.com/20fa771e874c7e368299ec7cba6776a810d40aaf/components/subresource_filter/content/browser/verified_ruleset_dealer.cc
[modify] https://crrev.com/20fa771e874c7e368299ec7cba6776a810d40aaf/components/subresource_filter/content/browser/verified_ruleset_dealer.h
[modify] https://crrev.com/20fa771e874c7e368299ec7cba6776a810d40aaf/components/subresource_filter/content/browser/verified_ruleset_dealer_unittest.cc
[modify] https://crrev.com/20fa771e874c7e368299ec7cba6776a810d40aaf/components/subresource_filter/core/browser/ruleset_service.cc
[modify] https://crrev.com/20fa771e874c7e368299ec7cba6776a810d40aaf/components/subresource_filter/core/browser/ruleset_service.h
[modify] https://crrev.com/20fa771e874c7e368299ec7cba6776a810d40aaf/components/subresource_filter/core/browser/ruleset_service_delegate.h
[modify] https://crrev.com/20fa771e874c7e368299ec7cba6776a810d40aaf/components/subresource_filter/core/browser/ruleset_service_unittest.cc
[modify] https://crrev.com/20fa771e874c7e368299ec7cba6776a810d40aaf/components/subresource_filter/core/common/indexed_ruleset.cc
[modify] https://crrev.com/20fa771e874c7e368299ec7cba6776a810d40aaf/components/subresource_filter/core/common/indexed_ruleset.h
[modify] https://crrev.com/20fa771e874c7e368299ec7cba6776a810d40aaf/components/subresource_filter/core/common/indexed_ruleset_fuzzer.cc
[modify] https://crrev.com/20fa771e874c7e368299ec7cba6776a810d40aaf/components/subresource_filter/core/common/test_ruleset_creator.h
►

Sign in to add a comment