Commit 6f3cd908 authored by guidou's avatar guidou Committed by Commit bot

Revert of Integrate registry_hash_store_contents with the rest of tracked...

Revert of Integrate registry_hash_store_contents with the rest of tracked prefs. (patchset #24 id:460001 of https://codereview.chromium.org/2204943002/ )

Reason for revert:
This CL is suspect of breaking WebRTC Windows bots.
I will reland if it doesn't fix the problem.

See, for example, https://build.chromium.org/p/chromium.webrtc/builders/Win7%20Tester/builds/30007/steps/browser_tests/logs/stdio

[4936:6236:0930/181348:INFO:webrtc_video_quality_browsertest.cc(245)] Running "C:\b\depot_tools\python276_bin\python.exe" -u "C:\b\c\b\Win7_Tester\src\third_party/webrtc/tools/compare_videos.py" --label=720p_VP9 --ref_video "C:\b\c\b\Win7_Tester\src\chrome\test\data\webrtc/resources\reference_video_1280x720_30fps.yuv" --test_video "C:\Users\CHROME~1.LAB\AppData\Local\Temp\scoped_dir4936_9527\captured_video.yuv" --frame_analyzer "C:\b\c\b\Win7_Tester\src\out\Release\frame_analyzer.exe" --yuv_frame_width 1280 --yuv_frame_height 720 --zxing_path "C:\b\c\b\Win7_Tester\src\chrome\test\data\webrtc/resources\tools\win\zxing.exe" --ffmpeg_path "C:\b\c\b\Win7_Tester\src\chrome\test\data\webrtc/resources\tools\win\ffmpeg.exe" --stats_file "C:\Users\CHROME~1.LAB\AppData\Local\Temp\scoped_dir4936_9527\stats.txt"
[4936:6236:0930/181420:INFO:user_input_monitor_win.cc(171)] RegisterRawInputDevices() failed for RIDEV_REMOVE: The parameter is incorrect. (0x57)
[4936:6236:0930/181421:FATAL:json_pref_store.cc(385)] Check failed: !has_pending_write_callbacks_.
Backtrace:
	base::debug::StackTrace::StackTrace [0x02BA3CF7+23]
	logging::LogMessage::~LogMessage [0x02B5B061+49]
	JsonPrefStore::RegisterOnNextWriteSynchronousCallbacks [0x03668FDF+154]
	JsonPrefStore::SerializeData [0x03669601+179]
	base::ImportantFileWriter::DoScheduledWrite [0x02BE4B15+149]
	JsonPrefStore::CommitPendingWrite [0x0366803E+120]
	JsonPrefStore::~JsonPrefStore [0x03667DF8+21]
	scoped_refptr<TestingPrefStore>::Release [0x056591C1+23]
	SegregatedPrefStore::~SegregatedPrefStore [0x04359463+95]
	scoped_refptr<TestingPrefStore>::Release [0x056591C1+23]
	PrefService::~PrefService [0x03663ED9+141]
	syncable_prefs::PrefServiceSyncable::~PrefServiceSyncable [0x04088703+93]
	syncable_prefs::PrefServiceSyncable::`scalar deleting destructor' [0x04088714+11]
	ProfileImpl::~ProfileImpl [0x02C5706B+271]
	ProfileDestroyer::DestroyProfileWhenAppropriate [0x02D1A381+484]
	ProfileManager::ProfileInfo::~ProfileInfo [0x02C04D62+14]
	std::_Tree<std::_Tmap_traits<base::FilePath,linked_ptr<ProfileManager::ProfileInfo>,std::less<base::FilePath>,std::allocator<std::pair<base::FilePath const ,linked_ptr<ProfileManager::ProfileInfo> > >,0> >::_Erase [0x02B54889+41]
	std::_Tree<std::_Tmap_traits<base::FilePath,linked_ptr<ProfileManager::ProfileInfo>,std::less<base::FilePath>,std::allocator<std::pair<base::FilePath const ,linked_ptr<ProfileManager::ProfileInfo> > >,0> >::clear [0x02B548B2+13]
	std::_Tree<std::_Tmap_traits<base::FilePath,linked_ptr<ProfileManager::ProfileInfo>,std::less<base::FilePath>,std::allocator<std::pair<base::FilePath const ,linked_ptr<ProfileManager::ProfileInfo> > >,0> >::~_Tree<std::_Tmap_traits<base::FilePath,linked_p [0x02C04D0E+21]
	ProfileManager::~ProfileManager [0x02C04DAF+54]
	BrowserProcessImpl::StartTearDown [0x02DD70E9+458]
	ChromeBrowserMainParts::PostMainMessageLoopRun [0x02E333B0+304]
	content::BrowserMainLoop::ShutdownThreadsAndCleanUp [0x024E327A+350]
	content::BrowserMainRunnerImpl::Shutdown [0x024E4498+632]
	content::BrowserMain [0x024DFE41+139]
	content::RunNamedProcessTypeMain [0x02B46FAB+206]
	content::ContentMainRunnerImpl::Run [0x02B46EAC+274]
	content::ContentMain [0x02B46275+35]
	content::BrowserTestBase::SetUp [0x02F73565+964]
	InProcessBrowserTest::SetUp [0x02BF45D7+268]
	testing::internal::HandleExceptionsInMethodIfSupported<testing::TestCase,void> [0x033F7AB9+32]
	testing::Test::Run [0x033FEAA3+51]
	testing::TestCase::Run [0x033FEB79+133]
	testing::internal::UnitTestImpl::RunAllTests [0x033FEEF8+433]
	testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl,bool> [0x033F7AFD+32]
	testing::UnitTest::Run [0x033FED22+133]
	base::TestSuite::Run [0x02BFD110+95]
	ChromeTestSuiteRunner::RunTestSuite [0x052456CE+40]
	content::LaunchTests [0x02F6D485+585]
	LaunchChromeTests [0x052456A1+49]
	main [0x052454BA+63]
	__scrt_common_main_seh [0x0520AD1F+249] (f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:253)
	BaseThreadInitThunk [0x75A0338A+18]
	RtlInitializeExceptionChain [0x77899902+99]
	RtlInitializeExceptionChain [0x778998D5+54]

Original issue's description:
> Integrate registry_hash_store_contents with the rest of tracked prefs.
>
> This change adds Windows-only logic to the PrefHashFilter such that it
> verifies preferences against MACs stored in the registry. Unlike the
> current tracked preference logic, this extra check does NOT reset
> settings.
>
> To avoid inconsistent state with the MACs in secure_preferences, we
> clear the registry MACs before writing secure_preferences, and write
> the registry MACs after the file is successfully written.
>
> BUG=624858
>
> Committed: https://crrev.com/4683dfcecfa72a73e00498d7c06fcf6716f8366c
> Cr-Commit-Position: refs/heads/master@{#422240}

TBR=gab@chromium.org,proberge@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=624858

Review-Url: https://codereview.chromium.org/2396443002
Cr-Commit-Position: refs/heads/master@{#422738}
parent e6b67c20
...@@ -11,7 +11,6 @@ ...@@ -11,7 +11,6 @@
#include "base/files/file_util.h" #include "base/files/file_util.h"
#include "base/json/json_file_value_serializer.h" #include "base/json/json_file_value_serializer.h"
#include "base/logging.h" #include "base/logging.h"
#include "base/memory/ptr_util.h"
#include "base/metrics/histogram_macros.h" #include "base/metrics/histogram_macros.h"
#include "base/sequenced_task_runner.h" #include "base/sequenced_task_runner.h"
#include "build/build_config.h" #include "build/build_config.h"
...@@ -24,11 +23,6 @@ ...@@ -24,11 +23,6 @@
#include "components/user_prefs/tracked/segregated_pref_store.h" #include "components/user_prefs/tracked/segregated_pref_store.h"
#include "components/user_prefs/tracked/tracked_preferences_migration.h" #include "components/user_prefs/tracked/tracked_preferences_migration.h"
#if defined(OS_WIN)
#include "chrome/installer/util/browser_distribution.h"
#include "components/user_prefs/tracked/registry_hash_store_contents_win.h"
#endif
namespace { namespace {
void RemoveValueSilently(const base::WeakPtr<JsonPrefStore> pref_store, void RemoveValueSilently(const base::WeakPtr<JsonPrefStore> pref_store,
...@@ -39,13 +33,6 @@ void RemoveValueSilently(const base::WeakPtr<JsonPrefStore> pref_store, ...@@ -39,13 +33,6 @@ void RemoveValueSilently(const base::WeakPtr<JsonPrefStore> pref_store,
} }
} }
#if defined(OS_WIN)
// Forces a different registry key to be used for storing preference validation
// MACs. See |SetPreferenceValidationRegistryPathForTesting|.
const base::string16* g_preference_validation_registry_path_for_testing =
nullptr;
#endif // OS_WIN
} // namespace } // namespace
// Preference tracking and protection is not required on platforms where other // Preference tracking and protection is not required on platforms where other
...@@ -90,15 +77,6 @@ void ProfilePrefStoreManager::ClearResetTime(PrefService* pref_service) { ...@@ -90,15 +77,6 @@ void ProfilePrefStoreManager::ClearResetTime(PrefService* pref_service) {
PrefHashFilter::ClearResetTime(pref_service); PrefHashFilter::ClearResetTime(pref_service);
} }
#if defined(OS_WIN)
// static
void ProfilePrefStoreManager::SetPreferenceValidationRegistryPathForTesting(
const base::string16* path) {
DCHECK(!path->empty());
g_preference_validation_registry_path_for_testing = path;
}
#endif // OS_WIN
PersistentPrefStore* ProfilePrefStoreManager::CreateProfilePrefStore( PersistentPrefStore* ProfilePrefStoreManager::CreateProfilePrefStore(
const scoped_refptr<base::SequencedTaskRunner>& io_task_runner, const scoped_refptr<base::SequencedTaskRunner>& io_task_runner,
const base::Closure& on_reset_on_load, const base::Closure& on_reset_on_load,
...@@ -130,14 +108,12 @@ PersistentPrefStore* ProfilePrefStoreManager::CreateProfilePrefStore( ...@@ -130,14 +108,12 @@ PersistentPrefStore* ProfilePrefStoreManager::CreateProfilePrefStore(
} }
std::unique_ptr<PrefHashFilter> unprotected_pref_hash_filter( std::unique_ptr<PrefHashFilter> unprotected_pref_hash_filter(
new PrefHashFilter(GetPrefHashStore(false), new PrefHashFilter(GetPrefHashStore(false), unprotected_configuration,
GetExternalVerificationPrefHashStorePair(), base::Closure(), validation_delegate,
unprotected_configuration, base::Closure(), reporting_ids_count_, false));
validation_delegate, reporting_ids_count_, false));
std::unique_ptr<PrefHashFilter> protected_pref_hash_filter(new PrefHashFilter( std::unique_ptr<PrefHashFilter> protected_pref_hash_filter(new PrefHashFilter(
GetPrefHashStore(true), GetExternalVerificationPrefHashStorePair(), GetPrefHashStore(true), protected_configuration, on_reset_on_load,
protected_configuration, on_reset_on_load, validation_delegate, validation_delegate, reporting_ids_count_, true));
reporting_ids_count_, true));
PrefHashFilter* raw_unprotected_pref_hash_filter = PrefHashFilter* raw_unprotected_pref_hash_filter =
unprotected_pref_hash_filter.get(); unprotected_pref_hash_filter.get();
...@@ -183,10 +159,11 @@ bool ProfilePrefStoreManager::InitializePrefsFromMasterPrefs( ...@@ -183,10 +159,11 @@ bool ProfilePrefStoreManager::InitializePrefsFromMasterPrefs(
copy.reset(master_prefs.DeepCopy()); copy.reset(master_prefs.DeepCopy());
to_serialize = copy.get(); to_serialize = copy.get();
PrefHashFilter(GetPrefHashStore(false), PrefHashFilter(GetPrefHashStore(false),
GetExternalVerificationPrefHashStorePair(), tracking_configuration_,
tracking_configuration_, base::Closure(), NULL, base::Closure(),
reporting_ids_count_, false) NULL,
.Initialize(copy.get()); reporting_ids_count_,
false).Initialize(copy.get());
} }
// This will write out to a single combined file which will be immediately // This will write out to a single combined file which will be immediately
...@@ -212,23 +189,3 @@ std::unique_ptr<PrefHashStore> ProfilePrefStoreManager::GetPrefHashStore( ...@@ -212,23 +189,3 @@ std::unique_ptr<PrefHashStore> ProfilePrefStoreManager::GetPrefHashStore(
return std::unique_ptr<PrefHashStore>( return std::unique_ptr<PrefHashStore>(
new PrefHashStoreImpl(seed_, device_id_, use_super_mac)); new PrefHashStoreImpl(seed_, device_id_, use_super_mac));
} }
std::pair<std::unique_ptr<PrefHashStore>, std::unique_ptr<HashStoreContents>>
ProfilePrefStoreManager::GetExternalVerificationPrefHashStorePair() {
DCHECK(kPlatformSupportsPreferenceTracking);
#if defined(OS_WIN)
return std::make_pair(
base::MakeUnique<PrefHashStoreImpl>(
"ChromeRegistryHashStoreValidationSeed", device_id_,
false /* use_super_mac */),
g_preference_validation_registry_path_for_testing
? base::MakeUnique<RegistryHashStoreContentsWin>(
*g_preference_validation_registry_path_for_testing,
profile_path_.BaseName().LossyDisplayName())
: base::MakeUnique<RegistryHashStoreContentsWin>(
BrowserDistribution::GetDistribution()->GetRegistryPath(),
profile_path_.BaseName().LossyDisplayName()));
#else
return std::make_pair(nullptr, nullptr);
#endif
}
...@@ -16,7 +16,6 @@ ...@@ -16,7 +16,6 @@
#include "base/memory/ref_counted.h" #include "base/memory/ref_counted.h"
#include "components/user_prefs/tracked/pref_hash_filter.h" #include "components/user_prefs/tracked/pref_hash_filter.h"
class HashStoreContents;
class PersistentPrefStore; class PersistentPrefStore;
class PrefHashStore; class PrefHashStore;
class PrefService; class PrefService;
...@@ -71,14 +70,6 @@ class ProfilePrefStoreManager { ...@@ -71,14 +70,6 @@ class ProfilePrefStoreManager {
// was built by ProfilePrefStoreManager. // was built by ProfilePrefStoreManager.
static void ClearResetTime(PrefService* pref_service); static void ClearResetTime(PrefService* pref_service);
#if defined(OS_WIN)
// Call before startup tasks kick in to use a different registry path for
// storing and validating tracked preference MACs. Callers are responsible
// for ensuring that the key is deleted on shutdown. For testing only.
static void SetPreferenceValidationRegistryPathForTesting(
const base::string16* path);
#endif
// Creates a PersistentPrefStore providing access to the user preferences of // Creates a PersistentPrefStore providing access to the user preferences of
// the managed profile. If |on_reset| is provided, it will be invoked if a // the managed profile. If |on_reset| is provided, it will be invoked if a
// reset occurs as a result of loading the profile's prefs. // reset occurs as a result of loading the profile's prefs.
...@@ -108,12 +99,6 @@ class ProfilePrefStoreManager { ...@@ -108,12 +99,6 @@ class ProfilePrefStoreManager {
// TrustedInitialized). // TrustedInitialized).
std::unique_ptr<PrefHashStore> GetPrefHashStore(bool use_super_mac); std::unique_ptr<PrefHashStore> GetPrefHashStore(bool use_super_mac);
// Returns a PrefHashStore and HashStoreContents which can be be used for
// extra out-of-band verifications, or nullptrs if not available on this
// platform.
std::pair<std::unique_ptr<PrefHashStore>, std::unique_ptr<HashStoreContents>>
GetExternalVerificationPrefHashStorePair();
const base::FilePath profile_path_; const base::FilePath profile_path_;
const std::vector<PrefHashFilter::TrackedPreferenceMetadata> const std::vector<PrefHashFilter::TrackedPreferenceMetadata>
tracking_configuration_; tracking_configuration_;
......
...@@ -38,10 +38,6 @@ ...@@ -38,10 +38,6 @@
#include "chromeos/chromeos_switches.h" #include "chromeos/chromeos_switches.h"
#endif #endif
#if defined(OS_WIN)
#include "base/test/test_reg_util_win.h"
#endif
namespace { namespace {
// Extension ID of chrome/test/data/extensions/good.crx // Extension ID of chrome/test/data/extensions/good.crx
...@@ -60,27 +56,14 @@ enum AllowedBuckets { ...@@ -60,27 +56,14 @@ enum AllowedBuckets {
ALLOW_ANY ALLOW_ANY
}; };
#if defined(OS_WIN)
base::string16 GetRegistryPathForTestProfile() {
base::FilePath profile_dir;
EXPECT_TRUE(PathService::Get(chrome::DIR_USER_DATA, &profile_dir));
return L"SOFTWARE\\Chromium\\PrefHashBrowserTest\\" +
profile_dir.BaseName().value();
}
#endif
// Returns the number of times |histogram_name| was reported so far; adding the // Returns the number of times |histogram_name| was reported so far; adding the
// results of the first 100 buckets (there are only ~19 reporting IDs as of this // results of the first 100 buckets (there are only ~19 reporting IDs as of this
// writing; varies depending on the platform). |allowed_buckets| hints at extra // writing; varies depending on the platform). |allowed_buckets| hints at extra
// requirements verified in this method (see AllowedBuckets for details). // requirements verified in this method (see AllowedBuckets for details).
int GetTrackedPrefHistogramCount(const char* histogram_name, int GetTrackedPrefHistogramCount(const char* histogram_name,
const char* histogram_suffix,
int allowed_buckets) { int allowed_buckets) {
std::string full_histogram_name(histogram_name);
if (*histogram_suffix)
full_histogram_name.append(".").append(histogram_suffix);
const base::HistogramBase* histogram = const base::HistogramBase* histogram =
base::StatisticsRecorder::FindHistogram(full_histogram_name); base::StatisticsRecorder::FindHistogram(histogram_name);
if (!histogram) if (!histogram)
return 0; return 0;
...@@ -99,13 +82,6 @@ int GetTrackedPrefHistogramCount(const char* histogram_name, ...@@ -99,13 +82,6 @@ int GetTrackedPrefHistogramCount(const char* histogram_name,
return sum; return sum;
} }
// Helper function to call GetTrackedPrefHistogramCount with no external
// validation suffix.
int GetTrackedPrefHistogramCount(const char* histogram_name,
int allowed_buckets) {
return GetTrackedPrefHistogramCount(histogram_name, "", allowed_buckets);
}
std::unique_ptr<base::DictionaryValue> ReadPrefsDictionary( std::unique_ptr<base::DictionaryValue> ReadPrefsDictionary(
const base::FilePath& pref_file) { const base::FilePath& pref_file) {
JSONFileValueDeserializer deserializer(pref_file); JSONFileValueDeserializer deserializer(pref_file);
...@@ -125,16 +101,6 @@ std::unique_ptr<base::DictionaryValue> ReadPrefsDictionary( ...@@ -125,16 +101,6 @@ std::unique_ptr<base::DictionaryValue> ReadPrefsDictionary(
static_cast<base::DictionaryValue*>(prefs.release())); static_cast<base::DictionaryValue*>(prefs.release()));
} }
// Returns whether external validation is supported on the platform through
// storing MACs in the registry.
bool SupportsRegistryValidation() {
#if defined(OS_WIN)
return true;
#else
return false;
#endif
}
#define PREF_HASH_BROWSER_TEST(fixture, test_name) \ #define PREF_HASH_BROWSER_TEST(fixture, test_name) \
IN_PROC_BROWSER_TEST_P(fixture, PRE_##test_name) { \ IN_PROC_BROWSER_TEST_P(fixture, PRE_##test_name) { \
SetupPreferences(); \ SetupPreferences(); \
...@@ -267,44 +233,6 @@ class PrefHashBrowserTestBase ...@@ -267,44 +233,6 @@ class PrefHashBrowserTestBase
// Bots are on a domain, turn off the domain check for settings hardening in // Bots are on a domain, turn off the domain check for settings hardening in
// order to be able to test all SettingsEnforcement groups. // order to be able to test all SettingsEnforcement groups.
chrome_prefs::DisableDomainCheckForTesting(); chrome_prefs::DisableDomainCheckForTesting();
#if defined(OS_WIN)
// Avoid polluting prefs for the user and the bots by writing to a specific
// testing registry path.
registry_key_for_external_validation_ = GetRegistryPathForTestProfile();
ProfilePrefStoreManager::SetPreferenceValidationRegistryPathForTesting(
&registry_key_for_external_validation_);
// Keys should be unique, but to avoid flakes in the long run make sure an
// identical test key wasn't left behind by a previous test.
if (IsPRETest()) {
base::win::RegKey key;
if (key.Open(HKEY_CURRENT_USER,
registry_key_for_external_validation_.c_str(),
KEY_SET_VALUE | KEY_WOW64_32KEY) == ERROR_SUCCESS) {
LONG result = key.DeleteKey(L"");
ASSERT_TRUE(result == ERROR_SUCCESS || result == ERROR_FILE_NOT_FOUND);
}
}
#endif
}
void TearDown() override {
#if defined(OS_WIN)
// When done, delete the Registry key to avoid polluting the registry.
// TODO(proberge): it would be nice to delete keys from interrupted tests
// as well.
if (!IsPRETest()) {
base::string16 registry_key = GetRegistryPathForTestProfile();
base::win::RegKey key;
if (key.Open(HKEY_CURRENT_USER, registry_key.c_str(),
KEY_SET_VALUE | KEY_WOW64_32KEY) == ERROR_SUCCESS) {
LONG result = key.DeleteKey(L"");
ASSERT_TRUE(result == ERROR_SUCCESS || result == ERROR_FILE_NOT_FOUND);
}
}
#endif
ExtensionBrowserTest::TearDown();
} }
// In the PRE_ test, find the number of tracked preferences that were // In the PRE_ test, find the number of tracked preferences that were
...@@ -338,23 +266,6 @@ class PrefHashBrowserTestBase ...@@ -338,23 +266,6 @@ class PrefHashBrowserTestBase
EXPECT_EQ(protection_level_ > PROTECTION_DISABLED_ON_PLATFORM ? 1 : 0, EXPECT_EQ(protection_level_ > PROTECTION_DISABLED_ON_PLATFORM ? 1 : 0,
num_split_tracked_prefs); num_split_tracked_prefs);
if (SupportsRegistryValidation()) {
// Same checks as above, but for the registry.
num_tracked_prefs_ = GetTrackedPrefHistogramCount(
user_prefs::tracked::kTrackedPrefHistogramNullInitialized,
user_prefs::tracked::kTrackedPrefRegistryValidationSuffix,
ALLOW_ANY);
EXPECT_EQ(protection_level_ > PROTECTION_DISABLED_ON_PLATFORM,
num_tracked_prefs_ > 0);
int num_split_tracked_prefs = GetTrackedPrefHistogramCount(
user_prefs::tracked::kTrackedPrefHistogramUnchanged,
user_prefs::tracked::kTrackedPrefRegistryValidationSuffix,
BEGIN_ALLOW_SINGLE_BUCKET + 5);
EXPECT_EQ(protection_level_ > PROTECTION_DISABLED_ON_PLATFORM ? 1 : 0,
num_split_tracked_prefs);
}
num_tracked_prefs_ += num_split_tracked_prefs; num_tracked_prefs_ += num_split_tracked_prefs;
std::string num_tracked_prefs_str = base::IntToString(num_tracked_prefs_); std::string num_tracked_prefs_str = base::IntToString(num_tracked_prefs_);
...@@ -442,10 +353,6 @@ class PrefHashBrowserTestBase ...@@ -442,10 +353,6 @@ class PrefHashBrowserTestBase
} }
int num_tracked_prefs_; int num_tracked_prefs_;
#if defined(OS_WIN)
base::string16 registry_key_for_external_validation_;
#endif
}; };
} // namespace } // namespace
...@@ -500,17 +407,6 @@ class PrefHashBrowserTestUnchangedDefault : public PrefHashBrowserTestBase { ...@@ -500,17 +407,6 @@ class PrefHashBrowserTestUnchangedDefault : public PrefHashBrowserTestBase {
0, GetTrackedPrefHistogramCount( 0, GetTrackedPrefHistogramCount(
user_prefs::tracked::kTrackedPrefHistogramMigratedLegacyDeviceId, user_prefs::tracked::kTrackedPrefHistogramMigratedLegacyDeviceId,
ALLOW_NONE)); ALLOW_NONE));
if (SupportsRegistryValidation()) {
// Expect all prefs to be reported as Unchanged.
EXPECT_EQ(protection_level_ > PROTECTION_DISABLED_ON_PLATFORM
? num_tracked_prefs()
: 0,
GetTrackedPrefHistogramCount(
user_prefs::tracked::kTrackedPrefHistogramUnchanged,
user_prefs::tracked::kTrackedPrefRegistryValidationSuffix,
ALLOW_ANY));
}
} }
}; };
...@@ -601,15 +497,6 @@ class PrefHashBrowserTestClearedAtomic : public PrefHashBrowserTestBase { ...@@ -601,15 +497,6 @@ class PrefHashBrowserTestClearedAtomic : public PrefHashBrowserTestBase {
0, GetTrackedPrefHistogramCount( 0, GetTrackedPrefHistogramCount(
user_prefs::tracked::kTrackedPrefHistogramMigratedLegacyDeviceId, user_prefs::tracked::kTrackedPrefHistogramMigratedLegacyDeviceId,
ALLOW_NONE)); ALLOW_NONE));
if (SupportsRegistryValidation()) {
// Expect homepage clearance to have been noticed by registry validation.
EXPECT_EQ(protection_level_ > PROTECTION_DISABLED_ON_PLATFORM ? 1 : 0,
GetTrackedPrefHistogramCount(
user_prefs::tracked::kTrackedPrefHistogramCleared,
user_prefs::tracked::kTrackedPrefRegistryValidationSuffix,
BEGIN_ALLOW_SINGLE_BUCKET + 2));
}
} }
}; };
...@@ -732,18 +619,6 @@ class PrefHashBrowserTestUntrustedInitialized : public PrefHashBrowserTestBase { ...@@ -732,18 +619,6 @@ class PrefHashBrowserTestUntrustedInitialized : public PrefHashBrowserTestBase {
0, GetTrackedPrefHistogramCount( 0, GetTrackedPrefHistogramCount(
user_prefs::tracked::kTrackedPrefHistogramMigratedLegacyDeviceId, user_prefs::tracked::kTrackedPrefHistogramMigratedLegacyDeviceId,
ALLOW_NONE)); ALLOW_NONE));
if (SupportsRegistryValidation()) {
// The MACs have been cleared but the preferences have not been tampered.
// The registry should report all prefs as unchanged.
EXPECT_EQ(protection_level_ > PROTECTION_DISABLED_ON_PLATFORM
? num_tracked_prefs()
: 0,
GetTrackedPrefHistogramCount(
user_prefs::tracked::kTrackedPrefHistogramUnchanged,
user_prefs::tracked::kTrackedPrefRegistryValidationSuffix,
ALLOW_ANY));
}
} }
}; };
...@@ -834,15 +709,6 @@ class PrefHashBrowserTestChangedAtomic : public PrefHashBrowserTestBase { ...@@ -834,15 +709,6 @@ class PrefHashBrowserTestChangedAtomic : public PrefHashBrowserTestBase {
0, GetTrackedPrefHistogramCount( 0, GetTrackedPrefHistogramCount(
user_prefs::tracked::kTrackedPrefHistogramMigratedLegacyDeviceId, user_prefs::tracked::kTrackedPrefHistogramMigratedLegacyDeviceId,
ALLOW_NONE)); ALLOW_NONE));
if (SupportsRegistryValidation()) {
// Expect a single Changed event for tracked pref #4 (startup URLs).
EXPECT_EQ(protection_level_ > PROTECTION_DISABLED_ON_PLATFORM ? 1 : 0,
GetTrackedPrefHistogramCount(
user_prefs::tracked::kTrackedPrefHistogramChanged,
user_prefs::tracked::kTrackedPrefRegistryValidationSuffix,
BEGIN_ALLOW_SINGLE_BUCKET + 4));
}
} }
}; };
...@@ -940,16 +806,6 @@ class PrefHashBrowserTestChangedSplitPref : public PrefHashBrowserTestBase { ...@@ -940,16 +806,6 @@ class PrefHashBrowserTestChangedSplitPref : public PrefHashBrowserTestBase {
0, GetTrackedPrefHistogramCount( 0, GetTrackedPrefHistogramCount(
user_prefs::tracked::kTrackedPrefHistogramMigratedLegacyDeviceId, user_prefs::tracked::kTrackedPrefHistogramMigratedLegacyDeviceId,
ALLOW_NONE)); ALLOW_NONE));
if (SupportsRegistryValidation()) {
// Expect that the registry validation caught the invalid MAC in split
// pref #5 (extensions).
EXPECT_EQ(protection_level_ > PROTECTION_DISABLED_ON_PLATFORM ? 1 : 0,
GetTrackedPrefHistogramCount(
user_prefs::tracked::kTrackedPrefHistogramChanged,
user_prefs::tracked::kTrackedPrefRegistryValidationSuffix,
BEGIN_ALLOW_SINGLE_BUCKET + 5));
}
} }
}; };
...@@ -1023,17 +879,6 @@ class PrefHashBrowserTestUntrustedAdditionToPrefs ...@@ -1023,17 +879,6 @@ class PrefHashBrowserTestUntrustedAdditionToPrefs
0, GetTrackedPrefHistogramCount( 0, GetTrackedPrefHistogramCount(
user_prefs::tracked::kTrackedPrefHistogramMigratedLegacyDeviceId, user_prefs::tracked::kTrackedPrefHistogramMigratedLegacyDeviceId,
ALLOW_NONE)); ALLOW_NONE));
if (SupportsRegistryValidation()) {
EXPECT_EQ((protection_level_ > PROTECTION_DISABLED_ON_PLATFORM &&
protection_level_ < PROTECTION_ENABLED_BASIC)
? changed_expected
: 0,
GetTrackedPrefHistogramCount(
user_prefs::tracked::kTrackedPrefHistogramChanged,
user_prefs::tracked::kTrackedPrefRegistryValidationSuffix,
BEGIN_ALLOW_SINGLE_BUCKET + 3));
}
} }
}; };
...@@ -1108,66 +953,8 @@ class PrefHashBrowserTestUntrustedAdditionToPrefsAfterWipe ...@@ -1108,66 +953,8 @@ class PrefHashBrowserTestUntrustedAdditionToPrefsAfterWipe
0, GetTrackedPrefHistogramCount( 0, GetTrackedPrefHistogramCount(
user_prefs::tracked::kTrackedPrefHistogramMigratedLegacyDeviceId, user_prefs::tracked::kTrackedPrefHistogramMigratedLegacyDeviceId,
ALLOW_NONE)); ALLOW_NONE));
if (SupportsRegistryValidation()) {
EXPECT_EQ(changed_expected,
GetTrackedPrefHistogramCount(
user_prefs::tracked::kTrackedPrefHistogramChanged,
user_prefs::tracked::kTrackedPrefRegistryValidationSuffix,
BEGIN_ALLOW_SINGLE_BUCKET + 2));
EXPECT_EQ(cleared_expected,
GetTrackedPrefHistogramCount(
user_prefs::tracked::kTrackedPrefHistogramCleared,
user_prefs::tracked::kTrackedPrefRegistryValidationSuffix,
BEGIN_ALLOW_SINGLE_BUCKET + 2));
}
} }
}; };
PREF_HASH_BROWSER_TEST(PrefHashBrowserTestUntrustedAdditionToPrefsAfterWipe, PREF_HASH_BROWSER_TEST(PrefHashBrowserTestUntrustedAdditionToPrefsAfterWipe,
UntrustedAdditionToPrefsAfterWipe); UntrustedAdditionToPrefsAfterWipe);
#if defined(OS_WIN)
class PrefHashBrowserTestRegistryValidationFailure
: public PrefHashBrowserTestBase {
public:
void SetupPreferences() override {
profile()->GetPrefs()->SetString(prefs::kHomePage, "http://example.com");
}
void AttackPreferencesOnDisk(
base::DictionaryValue* unprotected_preferences,
base::DictionaryValue* protected_preferences) override {
base::string16 registry_key =
GetRegistryPathForTestProfile() + L"\\PreferenceMACs\\Default";
base::win::RegKey key;
ASSERT_EQ(ERROR_SUCCESS, key.Open(HKEY_CURRENT_USER, registry_key.c_str(),
KEY_SET_VALUE | KEY_WOW64_32KEY));
// An incorrect hash should still have the correct size.
ASSERT_EQ(ERROR_SUCCESS,
key.WriteValue(L"homepage", base::string16(64, 'A').c_str()));
}
void VerifyReactionToPrefAttack() override {
EXPECT_EQ(
protection_level_ > PROTECTION_DISABLED_ON_PLATFORM
? num_tracked_prefs()
: 0,
GetTrackedPrefHistogramCount(
user_prefs::tracked::kTrackedPrefHistogramUnchanged, ALLOW_ANY));
if (SupportsRegistryValidation()) {
// Expect that the registry validation caught the invalid MAC for pref #2
// (homepage).
EXPECT_EQ(protection_level_ > PROTECTION_DISABLED_ON_PLATFORM ? 1 : 0,
GetTrackedPrefHistogramCount(
user_prefs::tracked::kTrackedPrefHistogramChanged,
user_prefs::tracked::kTrackedPrefRegistryValidationSuffix,
BEGIN_ALLOW_SINGLE_BUCKET + 2));
}
}
};
PREF_HASH_BROWSER_TEST(PrefHashBrowserTestRegistryValidationFailure,
RegistryValidationFailure);
#endif
...@@ -28,16 +28,6 @@ void DictionaryHashStoreContents::RegisterProfilePrefs( ...@@ -28,16 +28,6 @@ void DictionaryHashStoreContents::RegisterProfilePrefs(
registry->RegisterStringPref(kSuperMACPref, std::string()); registry->RegisterStringPref(kSuperMACPref, std::string());
} }
bool DictionaryHashStoreContents::IsCopyable() const {
return false;
}
std::unique_ptr<HashStoreContents> DictionaryHashStoreContents::MakeCopy()
const {
NOTREACHED() << "DictionaryHashStoreContents does not support MakeCopy";
return nullptr;
}
base::StringPiece DictionaryHashStoreContents::GetUMASuffix() const { base::StringPiece DictionaryHashStoreContents::GetUMASuffix() const {
// To stay consistent with existing reported data, do not append a suffix // To stay consistent with existing reported data, do not append a suffix
// when reporting UMA stats for this content. // when reporting UMA stats for this content.
...@@ -131,4 +121,4 @@ base::DictionaryValue* DictionaryHashStoreContents::GetMutableContents( ...@@ -131,4 +121,4 @@ base::DictionaryValue* DictionaryHashStoreContents::GetMutableContents(
storage_->Set(kPreferenceMACs, macs_dict); storage_->Set(kPreferenceMACs, macs_dict);
} }
return macs_dict; return macs_dict;
} }
\ No newline at end of file
...@@ -31,8 +31,6 @@ class DictionaryHashStoreContents : public HashStoreContents { ...@@ -31,8 +31,6 @@ class DictionaryHashStoreContents : public HashStoreContents {
static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry); static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry);
// HashStoreContents implementation // HashStoreContents implementation
bool IsCopyable() const override;
std::unique_ptr<HashStoreContents> MakeCopy() const override;
base::StringPiece GetUMASuffix() const override; base::StringPiece GetUMASuffix() const override;
void Reset() override; void Reset() override;
bool GetMac(const std::string& path, std::string* out_value) override; bool GetMac(const std::string& path, std::string* out_value) override;
......
...@@ -26,15 +26,6 @@ class HashStoreContents { ...@@ -26,15 +26,6 @@ class HashStoreContents {
public: public:
virtual ~HashStoreContents() {} virtual ~HashStoreContents() {}
// Returns true if this implementation of HashStoreContents can be copied via
// MakeCopy().
virtual bool IsCopyable() const = 0;
// Returns a copy of this HashStoreContents. Must only be called on
// lightweight implementations (which return true from IsCopyable()) and only
// in scenarios where a copy cannot be avoided.
virtual std::unique_ptr<HashStoreContents> MakeCopy() const = 0;
// Returns the suffix to be appended to UMA histograms for this store type. // Returns the suffix to be appended to UMA histograms for this store type.
// The returned value must either be an empty string or one of the values in // The returned value must either be an empty string or one of the values in
// histograms.xml's TrackedPreferencesExternalValidators. // histograms.xml's TrackedPreferencesExternalValidators.
......
...@@ -8,10 +8,8 @@ ...@@ -8,10 +8,8 @@
#include <algorithm> #include <algorithm>
#include <utility> #include <utility>
#include "base/bind.h"
#include "base/logging.h" #include "base/logging.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/ptr_util.h"
#include "base/metrics/histogram_macros.h" #include "base/metrics/histogram_macros.h"
#include "base/strings/string_number_conversions.h" #include "base/strings/string_number_conversions.h"
#include "base/time/time.h" #include "base/time/time.h"
...@@ -53,27 +51,16 @@ void CleanupDeprecatedTrackedPreferences( ...@@ -53,27 +51,16 @@ void CleanupDeprecatedTrackedPreferences(
PrefHashFilter::PrefHashFilter( PrefHashFilter::PrefHashFilter(
std::unique_ptr<PrefHashStore> pref_hash_store, std::unique_ptr<PrefHashStore> pref_hash_store,
StoreContentsPair external_validation_hash_store_pair,
const std::vector<TrackedPreferenceMetadata>& tracked_preferences, const std::vector<TrackedPreferenceMetadata>& tracked_preferences,
const base::Closure& on_reset_on_load, const base::Closure& on_reset_on_load,
TrackedPreferenceValidationDelegate* delegate, TrackedPreferenceValidationDelegate* delegate,
size_t reporting_ids_count, size_t reporting_ids_count,
bool report_super_mac_validity) bool report_super_mac_validity)
: pref_hash_store_(std::move(pref_hash_store)), : pref_hash_store_(std::move(pref_hash_store)),
external_validation_hash_store_pair_(
external_validation_hash_store_pair.first
? base::make_optional(
std::move(external_validation_hash_store_pair))
: base::nullopt),
on_reset_on_load_(on_reset_on_load), on_reset_on_load_(on_reset_on_load),
report_super_mac_validity_(report_super_mac_validity) { report_super_mac_validity_(report_super_mac_validity) {
DCHECK(pref_hash_store_); DCHECK(pref_hash_store_);
DCHECK_GE(reporting_ids_count, tracked_preferences.size()); DCHECK_GE(reporting_ids_count, tracked_preferences.size());
// Verify that, if |external_validation_hash_store_pair_| is present, both its
// items are non-null.
DCHECK(!external_validation_hash_store_pair_.has_value() ||
(external_validation_hash_store_pair_->first &&
external_validation_hash_store_pair_->second));
for (size_t i = 0; i < tracked_preferences.size(); ++i) { for (size_t i = 0; i < tracked_preferences.size(); ++i) {
const TrackedPreferenceMetadata& metadata = tracked_preferences[i]; const TrackedPreferenceMetadata& metadata = tracked_preferences[i];
...@@ -152,7 +139,7 @@ void PrefHashFilter::Initialize(base::DictionaryValue* pref_store_contents) { ...@@ -152,7 +139,7 @@ void PrefHashFilter::Initialize(base::DictionaryValue* pref_store_contents) {
it != tracked_paths_.end(); ++it) { it != tracked_paths_.end(); ++it) {
const std::string& initialized_path = it->first; const std::string& initialized_path = it->first;
const TrackedPreference* initialized_preference = it->second; const TrackedPreference* initialized_preference = it->second;
const base::Value* value = nullptr; const base::Value* value = NULL;
pref_store_contents->Get(initialized_path, &value); pref_store_contents->Get(initialized_path, &value);
initialized_preference->OnNewValue(value, hash_store_transaction.get()); initialized_preference->OnNewValue(value, hash_store_transaction.get());
} }
...@@ -171,40 +158,31 @@ void PrefHashFilter::FilterUpdate(const std::string& path) { ...@@ -171,40 +158,31 @@ void PrefHashFilter::FilterUpdate(const std::string& path) {
// is too expensive (see perf regression @ http://crbug.com/331273). // is too expensive (see perf regression @ http://crbug.com/331273).
PrefFilter::OnWriteCallbackPair PrefHashFilter::FilterSerializeData( PrefFilter::OnWriteCallbackPair PrefHashFilter::FilterSerializeData(
base::DictionaryValue* pref_store_contents) { base::DictionaryValue* pref_store_contents) {
// Generate the callback pair before clearing |changed_paths_|.
PrefFilter::OnWriteCallbackPair callback_pair =
GetOnWriteSynchronousCallbacks(pref_store_contents);
if (!changed_paths_.empty()) { if (!changed_paths_.empty()) {
base::TimeTicks checkpoint = base::TimeTicks::Now(); base::TimeTicks checkpoint = base::TimeTicks::Now();
{ {
DictionaryHashStoreContents dictionary_contents(pref_store_contents); DictionaryHashStoreContents dictionary_contents(pref_store_contents);
std::unique_ptr<PrefHashStoreTransaction> hash_store_transaction( std::unique_ptr<PrefHashStoreTransaction> hash_store_transaction(
pref_hash_store_->BeginTransaction(&dictionary_contents)); pref_hash_store_->BeginTransaction(&dictionary_contents));
std::unique_ptr<PrefHashStoreTransaction>
external_validation_hash_store_transaction;
if (external_validation_hash_store_pair_) {
external_validation_hash_store_transaction =
external_validation_hash_store_pair_->first->BeginTransaction(
external_validation_hash_store_pair_->second.get());
}
for (ChangedPathsMap::const_iterator it = changed_paths_.begin(); for (ChangedPathsMap::const_iterator it = changed_paths_.begin();
it != changed_paths_.end(); ++it) { it != changed_paths_.end(); ++it) {
const std::string& changed_path = it->first; const std::string& changed_path = it->first;
const TrackedPreference* changed_preference = it->second; const TrackedPreference* changed_preference = it->second;
const base::Value* value = nullptr; const base::Value* value = NULL;
pref_store_contents->Get(changed_path, &value); pref_store_contents->Get(changed_path, &value);
changed_preference->OnNewValue(value, hash_store_transaction.get()); changed_preference->OnNewValue(value, hash_store_transaction.get());
} }
changed_paths_.clear(); changed_paths_.clear();
} }
// TODO(gab): Remove this histogram by Feb 21 2014; after sufficient timing
// data has been gathered from the wild to be confident this doesn't
// significantly affect performance on the UI thread.
UMA_HISTOGRAM_TIMES("Settings.FilterSerializeDataTime", UMA_HISTOGRAM_TIMES("Settings.FilterSerializeDataTime",
base::TimeTicks::Now() - checkpoint); base::TimeTicks::Now() - checkpoint);
} }
return callback_pair; // TODO(proberge): return callbacks if external validation is enabled.
return std::make_pair(base::Closure(), base::Callback<void(bool success)>());
} }
void PrefHashFilter::FinalizeFilterOnLoad( void PrefHashFilter::FinalizeFilterOnLoad(
...@@ -220,14 +198,6 @@ void PrefHashFilter::FinalizeFilterOnLoad( ...@@ -220,14 +198,6 @@ void PrefHashFilter::FinalizeFilterOnLoad(
std::unique_ptr<PrefHashStoreTransaction> hash_store_transaction( std::unique_ptr<PrefHashStoreTransaction> hash_store_transaction(
pref_hash_store_->BeginTransaction(&dictionary_contents)); pref_hash_store_->BeginTransaction(&dictionary_contents));
std::unique_ptr<PrefHashStoreTransaction>
external_validation_hash_store_transaction;
if (external_validation_hash_store_pair_) {
external_validation_hash_store_transaction =
external_validation_hash_store_pair_->first->BeginTransaction(
external_validation_hash_store_pair_->second.get());
}
CleanupDeprecatedTrackedPreferences( CleanupDeprecatedTrackedPreferences(
pref_store_contents.get(), hash_store_transaction.get()); pref_store_contents.get(), hash_store_transaction.get());
...@@ -238,9 +208,8 @@ void PrefHashFilter::FinalizeFilterOnLoad( ...@@ -238,9 +208,8 @@ void PrefHashFilter::FinalizeFilterOnLoad(
for (TrackedPreferencesMap::const_iterator it = tracked_paths_.begin(); for (TrackedPreferencesMap::const_iterator it = tracked_paths_.begin();
it != tracked_paths_.end(); ++it) { it != tracked_paths_.end(); ++it) {
if (it->second->EnforceAndReport( if (it->second->EnforceAndReport(pref_store_contents.get(),
pref_store_contents.get(), hash_store_transaction.get(), hash_store_transaction.get())) {
external_validation_hash_store_transaction.get())) {
did_reset = true; did_reset = true;
prefs_altered = true; prefs_altered = true;
} }
...@@ -259,114 +228,12 @@ void PrefHashFilter::FinalizeFilterOnLoad( ...@@ -259,114 +228,12 @@ void PrefHashFilter::FinalizeFilterOnLoad(
on_reset_on_load_.Run(); on_reset_on_load_.Run();
} }
// TODO(gab): Remove this histogram by Feb 21 2014; after sufficient timing
// data has been gathered from the wild to be confident this doesn't
// significantly affect startup.
UMA_HISTOGRAM_TIMES("Settings.FilterOnLoadTime", UMA_HISTOGRAM_TIMES("Settings.FilterOnLoadTime",
base::TimeTicks::Now() - checkpoint); base::TimeTicks::Now() - checkpoint);
post_filter_on_load_callback.Run(std::move(pref_store_contents), post_filter_on_load_callback.Run(std::move(pref_store_contents),
prefs_altered); prefs_altered);
} }
// static
void PrefHashFilter::ClearFromExternalStore(
HashStoreContents* external_validation_hash_store_contents,
const base::DictionaryValue* changed_paths_and_macs) {
DCHECK(!changed_paths_and_macs->empty());
for (base::DictionaryValue::Iterator it(*changed_paths_and_macs);
!it.IsAtEnd(); it.Advance()) {
external_validation_hash_store_contents->RemoveEntry(it.key());
}
}
// static
void PrefHashFilter::FlushToExternalStore(
std::unique_ptr<HashStoreContents> external_validation_hash_store_contents,
std::unique_ptr<base::DictionaryValue> changed_paths_and_macs,
bool write_success) {
DCHECK(!changed_paths_and_macs->empty());
DCHECK(external_validation_hash_store_contents);
if (!write_success)
return;
for (base::DictionaryValue::Iterator it(*changed_paths_and_macs);
!it.IsAtEnd(); it.Advance()) {
const std::string& changed_path = it.key();
const base::DictionaryValue* split_values = nullptr;
if (it.value().GetAsDictionary(&split_values)) {
for (base::DictionaryValue::Iterator inner_it(*split_values);
!inner_it.IsAtEnd(); inner_it.Advance()) {
std::string mac;
bool is_string = inner_it.value().GetAsString(&mac);
DCHECK(is_string);
external_validation_hash_store_contents->SetSplitMac(
changed_path, inner_it.key(), mac);
}
} else {
const base::StringValue* value_as_string;
bool is_string = it.value().GetAsString(&value_as_string);
DCHECK(is_string);
external_validation_hash_store_contents->SetMac(
changed_path, value_as_string->GetString());
}
}
}
PrefFilter::OnWriteCallbackPair PrefHashFilter::GetOnWriteSynchronousCallbacks(
base::DictionaryValue* pref_store_contents) {
if (changed_paths_.empty() || !external_validation_hash_store_pair_) {
return std::make_pair(base::Closure(),
base::Callback<void(bool success)>());
}
std::unique_ptr<base::DictionaryValue> changed_paths_macs =
base::MakeUnique<base::DictionaryValue>();
for (ChangedPathsMap::const_iterator it = changed_paths_.begin();
it != changed_paths_.end(); ++it) {
const std::string& changed_path = it->first;
const TrackedPreference* changed_preference = it->second;
switch (changed_preference->GetType()) {
case TrackedPreferenceType::ATOMIC: {
const base::Value* new_value = nullptr;
pref_store_contents->Get(changed_path, &new_value);
changed_paths_macs->SetStringWithoutPathExpansion(
changed_path,
external_validation_hash_store_pair_->first->ComputeMac(
changed_path, new_value));
break;
}
case TrackedPreferenceType::SPLIT: {
const base::DictionaryValue* dict_value = nullptr;
pref_store_contents->GetDictionary(changed_path, &dict_value);
changed_paths_macs->SetWithoutPathExpansion(
changed_path,
external_validation_hash_store_pair_->first->ComputeSplitMacs(
changed_path, dict_value));
break;
}
}
}
DCHECK(external_validation_hash_store_pair_->second->IsCopyable())
<< "External HashStoreContents must be copyable as it needs to be used "
"off-thread";
std::unique_ptr<HashStoreContents> hash_store_contents_copy =
external_validation_hash_store_pair_->second->MakeCopy();
// We can use raw pointers for the first callback instead of making more
// copies as it will be executed in sequence before the second callback,
// which owns the pointers.
HashStoreContents* raw_contents = hash_store_contents_copy.get();
base::DictionaryValue* raw_changed_paths_macs = changed_paths_macs.get();
return std::make_pair(
base::Bind(&ClearFromExternalStore, base::Unretained(raw_contents),
base::Unretained(raw_changed_paths_macs)),
base::Bind(&FlushToExternalStore, base::Passed(&hash_store_contents_copy),
base::Passed(&changed_paths_macs)));
}
...@@ -15,10 +15,7 @@ ...@@ -15,10 +15,7 @@
#include "base/callback.h" #include "base/callback.h"
#include "base/compiler_specific.h" #include "base/compiler_specific.h"
#include "base/containers/scoped_ptr_hash_map.h" #include "base/containers/scoped_ptr_hash_map.h"
#include "base/files/file_path.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/optional.h"
#include "components/user_prefs/tracked/hash_store_contents.h"
#include "components/user_prefs/tracked/interceptable_pref_filter.h" #include "components/user_prefs/tracked/interceptable_pref_filter.h"
#include "components/user_prefs/tracked/tracked_preference.h" #include "components/user_prefs/tracked/tracked_preference.h"
...@@ -67,9 +64,6 @@ class PrefHashFilter : public InterceptablePrefFilter { ...@@ -67,9 +64,6 @@ class PrefHashFilter : public InterceptablePrefFilter {
ValueType value_type; ValueType value_type;
}; };
using StoreContentsPair = std::pair<std::unique_ptr<PrefHashStore>,
std::unique_ptr<HashStoreContents>>;
// Constructs a PrefHashFilter tracking the specified |tracked_preferences| // Constructs a PrefHashFilter tracking the specified |tracked_preferences|
// using |pref_hash_store| to check/store hashes. An optional |delegate| is // using |pref_hash_store| to check/store hashes. An optional |delegate| is
// notified of the status of each preference as it is checked. // notified of the status of each preference as it is checked.
...@@ -79,11 +73,8 @@ class PrefHashFilter : public InterceptablePrefFilter { ...@@ -79,11 +73,8 @@ class PrefHashFilter : public InterceptablePrefFilter {
// than |tracked_preferences.size()|). If |report_super_mac_validity| is true, // than |tracked_preferences.size()|). If |report_super_mac_validity| is true,
// the state of the super MAC will be reported via UMA during // the state of the super MAC will be reported via UMA during
// FinalizeFilterOnLoad. // FinalizeFilterOnLoad.
// |external_validation_hash_store_pair_| will be used (if non-null) to
// perform extra validations without triggering resets.
PrefHashFilter( PrefHashFilter(
std::unique_ptr<PrefHashStore> pref_hash_store, std::unique_ptr<PrefHashStore> pref_hash_store,
StoreContentsPair external_validation_hash_store_pair_,
const std::vector<TrackedPreferenceMetadata>& tracked_preferences, const std::vector<TrackedPreferenceMetadata>& tracked_preferences,
const base::Closure& on_reset_on_load, const base::Closure& on_reset_on_load,
TrackedPreferenceValidationDelegate* delegate, TrackedPreferenceValidationDelegate* delegate,
...@@ -120,25 +111,6 @@ class PrefHashFilter : public InterceptablePrefFilter { ...@@ -120,25 +111,6 @@ class PrefHashFilter : public InterceptablePrefFilter {
std::unique_ptr<base::DictionaryValue> pref_store_contents, std::unique_ptr<base::DictionaryValue> pref_store_contents,
bool prefs_altered) override; bool prefs_altered) override;
// Helper function to generate FilterSerializeData()'s pre-write and
// post-write callbacks. The returned callbacks are thread-safe.
OnWriteCallbackPair GetOnWriteSynchronousCallbacks(
base::DictionaryValue* pref_store_contents);
// Clears the MACs contained in |external_validation_hash_store_contents|
// which are present in |paths_to_clear|.
static void ClearFromExternalStore(
HashStoreContents* external_validation_hash_store_contents,
const base::DictionaryValue* changed_paths_and_macs);
// Flushes the MACs contained in |changed_paths_and_mac| to
// external_hash_store_contents if |write_success|, otherwise discards the
// changes.
static void FlushToExternalStore(
std::unique_ptr<HashStoreContents> external_hash_store_contents,
std::unique_ptr<base::DictionaryValue> changed_paths_and_macs,
bool write_success);
// Callback to be invoked only once (and subsequently reset) on the next // Callback to be invoked only once (and subsequently reset) on the next
// FilterOnLoad event. It will be allowed to modify the |prefs| handed to // FilterOnLoad event. It will be allowed to modify the |prefs| handed to
// FilterOnLoad before handing them back to this PrefHashFilter. // FilterOnLoad before handing them back to this PrefHashFilter.
...@@ -149,18 +121,12 @@ class PrefHashFilter : public InterceptablePrefFilter { ...@@ -149,18 +121,12 @@ class PrefHashFilter : public InterceptablePrefFilter {
typedef base::ScopedPtrHashMap<std::string, typedef base::ScopedPtrHashMap<std::string,
std::unique_ptr<TrackedPreference>> std::unique_ptr<TrackedPreference>>
TrackedPreferencesMap; TrackedPreferencesMap;
// A map from changed paths to their corresponding TrackedPreferences (which // A map from changed paths to their corresponding TrackedPreferences (which
// aren't owned by this map). // aren't owned by this map).
typedef std::map<std::string, const TrackedPreference*> ChangedPathsMap; typedef std::map<std::string, const TrackedPreference*> ChangedPathsMap;
std::unique_ptr<PrefHashStore> pref_hash_store_; std::unique_ptr<PrefHashStore> pref_hash_store_;
// A store and contents on which to perform extra validations without
// triggering resets.
// Will be null if the platform does not support external validation.
const base::Optional<StoreContentsPair> external_validation_hash_store_pair_;
// Invoked if a reset occurs in a call to FilterOnLoad. // Invoked if a reset occurs in a call to FilterOnLoad.
const base::Closure on_reset_on_load_; const base::Closure on_reset_on_load_;
......
...@@ -18,7 +18,7 @@ ...@@ -18,7 +18,7 @@
#include "base/compiler_specific.h" #include "base/compiler_specific.h"
#include "base/logging.h" #include "base/logging.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/ptr_util.h" #include "base/memory/ref_counted.h"
#include "base/metrics/histogram_base.h" #include "base/metrics/histogram_base.h"
#include "base/metrics/histogram_samples.h" #include "base/metrics/histogram_samples.h"
#include "base/metrics/statistics_recorder.h" #include "base/metrics/statistics_recorder.h"
...@@ -387,189 +387,6 @@ std::vector<PrefHashFilter::TrackedPreferenceMetadata> GetConfiguration( ...@@ -387,189 +387,6 @@ std::vector<PrefHashFilter::TrackedPreferenceMetadata> GetConfiguration(
return configuration; return configuration;
} }
class MockHashStoreContents : public HashStoreContents {
public:
MockHashStoreContents(){};
// Returns the number of hashes stored.
size_t stored_hashes_count() const { return dictionary_.size(); }
// Returns the number of paths cleared.
size_t cleared_paths_count() const { return removed_entries_.size(); }
// Returns the stored MAC for an Atomic preference.
std::string GetStoredMac(const std::string& path) const;
// Returns the stored MAC for a Split preference.
std::string GetStoredSplitMac(const std::string& path,
const std::string& split_path) const;
// HashStoreContents implementation.
bool IsCopyable() const override;
std::unique_ptr<HashStoreContents> MakeCopy() const override;
base::StringPiece GetUMASuffix() const override;
void Reset() override;
bool GetMac(const std::string& path, std::string* out_value) override;
bool GetSplitMacs(const std::string& path,
std::map<std::string, std::string>* split_macs) override;
void SetMac(const std::string& path, const std::string& value) override;
void SetSplitMac(const std::string& path,
const std::string& split_path,
const std::string& value) override;
void ImportEntry(const std::string& path,
const base::Value* in_value) override;
bool RemoveEntry(const std::string& path) override;
const base::DictionaryValue* GetContents() const override;
std::string GetSuperMac() const override;
void SetSuperMac(const std::string& super_mac) override;
private:
MockHashStoreContents(MockHashStoreContents* origin_mock);
// Records calls to this mock's SetMac/SetSplitMac methods.
void RecordSetMac(const std::string& path, const std::string& mac) {
dictionary_.SetStringWithoutPathExpansion(path, mac);
}
void RecordSetSplitMac(const std::string& path,
const std::string& split_path,
const std::string& mac) {
base::DictionaryValue* mac_dict = nullptr;
dictionary_.GetDictionaryWithoutPathExpansion(path, &mac_dict);
if (!mac_dict) {
mac_dict = new base::DictionaryValue;
dictionary_.SetWithoutPathExpansion(path, mac_dict);
}
mac_dict->SetStringWithoutPathExpansion(split_path, mac);
}
// Records a call to this mock's RemoveEntry method.
void RecordRemoveEntry(const std::string& path) {
// Don't expect the same pref to be cleared more than once.
EXPECT_EQ(removed_entries_.end(), removed_entries_.find(path));
removed_entries_.insert(path);
}
base::DictionaryValue dictionary_;
std::set<std::string> removed_entries_;
// The code being tested copies its HashStoreContents for use in a callback
// which can be executed during shutdown. To be able to capture the behavior
// of the copy, we make it forward calls to the mock it was created from.
// Once set, |origin_mock_| must outlive this instance.
MockHashStoreContents* origin_mock_;
DISALLOW_COPY_AND_ASSIGN(MockHashStoreContents);
};
std::string MockHashStoreContents::GetStoredMac(const std::string& path) const {
const base::Value* out_value;
if (dictionary_.GetWithoutPathExpansion(path, &out_value)) {
const base::StringValue* value_as_string;
EXPECT_TRUE(out_value->GetAsString(&value_as_string));
return value_as_string->GetString();
}
return std::string();
}
std::string MockHashStoreContents::GetStoredSplitMac(
const std::string& path,
const std::string& split_path) const {
const base::Value* out_value;
if (dictionary_.GetWithoutPathExpansion(path, &out_value)) {
const base::DictionaryValue* value_as_dict;
EXPECT_TRUE(out_value->GetAsDictionary(&value_as_dict));
if (value_as_dict->GetWithoutPathExpansion(split_path, &out_value)) {
const base::StringValue* value_as_string;
EXPECT_TRUE(out_value->GetAsString(&value_as_string));
return value_as_string->GetString();
}
}
return std::string();
}
MockHashStoreContents::MockHashStoreContents(MockHashStoreContents* origin_mock)
: origin_mock_(origin_mock) {}
bool MockHashStoreContents::IsCopyable() const {
return true;
}
std::unique_ptr<HashStoreContents> MockHashStoreContents::MakeCopy() const {
// Return a new MockHashStoreContents which forwards all requests to this
// mock instance.
return std::unique_ptr<HashStoreContents>(
new MockHashStoreContents(const_cast<MockHashStoreContents*>(this)));
}
base::StringPiece MockHashStoreContents::GetUMASuffix() const {
return "Unused";
}
void MockHashStoreContents::Reset() {
ADD_FAILURE() << "Unexpected call.";
}
bool MockHashStoreContents::GetMac(const std::string& path,
std::string* out_value) {
ADD_FAILURE() << "Unexpected call.";
return false;
}
bool MockHashStoreContents::GetSplitMacs(
const std::string& path,
std::map<std::string, std::string>* split_macs) {
ADD_FAILURE() << "Unexpected call.";
return false;
}
void MockHashStoreContents::SetMac(const std::string& path,
const std::string& value) {
if (origin_mock_)
origin_mock_->RecordSetMac(path, value);
else
RecordSetMac(path, value);
}
void MockHashStoreContents::SetSplitMac(const std::string& path,
const std::string& split_path,
const std::string& value) {
if (origin_mock_)
origin_mock_->RecordSetSplitMac(path, split_path, value);
else
RecordSetSplitMac(path, split_path, value);
}
void MockHashStoreContents::ImportEntry(const std::string& path,
const base::Value* in_value) {
ADD_FAILURE() << "Unexpected call.";
}
bool MockHashStoreContents::RemoveEntry(const std::string& path) {
if (origin_mock_)
origin_mock_->RecordRemoveEntry(path);
else
RecordRemoveEntry(path);
return true;
}
const base::DictionaryValue* MockHashStoreContents::GetContents() const {
ADD_FAILURE() << "Unexpected call.";
return nullptr;
}
std::string MockHashStoreContents::GetSuperMac() const {
ADD_FAILURE() << "Unexpected call.";
return std::string();
}
void MockHashStoreContents::SetSuperMac(const std::string& super_mac) {
ADD_FAILURE() << "Unexpected call.";
}
class PrefHashFilterTest class PrefHashFilterTest
: public testing::TestWithParam<PrefHashFilter::EnforcementLevel> { : public testing::TestWithParam<PrefHashFilter::EnforcementLevel> {
public: public:
...@@ -597,22 +414,9 @@ class PrefHashFilterTest ...@@ -597,22 +414,9 @@ class PrefHashFilterTest
PrefHashFilter::TrackedPreferenceMetadata>& configuration) { PrefHashFilter::TrackedPreferenceMetadata>& configuration) {
std::unique_ptr<MockPrefHashStore> temp_mock_pref_hash_store( std::unique_ptr<MockPrefHashStore> temp_mock_pref_hash_store(
new MockPrefHashStore); new MockPrefHashStore);
std::unique_ptr<MockPrefHashStore>
temp_mock_external_validation_pref_hash_store(new MockPrefHashStore);
std::unique_ptr<MockHashStoreContents>
temp_mock_external_validation_hash_store_contents(
new MockHashStoreContents);
mock_pref_hash_store_ = temp_mock_pref_hash_store.get(); mock_pref_hash_store_ = temp_mock_pref_hash_store.get();
mock_external_validation_pref_hash_store_ =
temp_mock_external_validation_pref_hash_store.get();
mock_external_validation_hash_store_contents_ =
temp_mock_external_validation_hash_store_contents.get();
pref_hash_filter_.reset(new PrefHashFilter( pref_hash_filter_.reset(new PrefHashFilter(
std::move(temp_mock_pref_hash_store), std::move(temp_mock_pref_hash_store), configuration,
PrefHashFilter::StoreContentsPair(
std::move(temp_mock_external_validation_pref_hash_store),
std::move(temp_mock_external_validation_hash_store_contents)),
configuration,
base::Bind(&PrefHashFilterTest::RecordReset, base::Unretained(this)), base::Bind(&PrefHashFilterTest::RecordReset, base::Unretained(this)),
&mock_validation_delegate_, arraysize(kTestTrackedPrefs), true)); &mock_validation_delegate_, arraysize(kTestTrackedPrefs), true));
} }
...@@ -638,8 +442,6 @@ class PrefHashFilterTest ...@@ -638,8 +442,6 @@ class PrefHashFilterTest
} }
MockPrefHashStore* mock_pref_hash_store_; MockPrefHashStore* mock_pref_hash_store_;
MockPrefHashStore* mock_external_validation_pref_hash_store_;
MockHashStoreContents* mock_external_validation_hash_store_contents_;
std::unique_ptr<base::DictionaryValue> pref_store_contents_; std::unique_ptr<base::DictionaryValue> pref_store_contents_;
MockValidationDelegate mock_validation_delegate_; MockValidationDelegate mock_validation_delegate_;
std::unique_ptr<PrefHashFilter> pref_hash_filter_; std::unique_ptr<PrefHashFilter> pref_hash_filter_;
...@@ -1261,120 +1063,6 @@ TEST_P(PrefHashFilterTest, DontResetReportOnly) { ...@@ -1261,120 +1063,6 @@ TEST_P(PrefHashFilterTest, DontResetReportOnly) {
} }
} }
TEST_P(PrefHashFilterTest, CallFilterSerializeDataCallbacks) {
base::DictionaryValue root_dict;
// Ownership of the following values is transfered to |root_dict|.
base::Value* int_value1 = new base::FundamentalValue(1);
base::Value* int_value2 = new base::FundamentalValue(2);
base::DictionaryValue* dict_value = new base::DictionaryValue;
dict_value->Set("a", new base::FundamentalValue(true));
root_dict.Set(kAtomicPref, int_value1);
root_dict.Set(kAtomicPref2, int_value2);
root_dict.Set(kSplitPref, dict_value);
// Skip updating kAtomicPref2.
pref_hash_filter_->FilterUpdate(kAtomicPref);
pref_hash_filter_->FilterUpdate(kSplitPref);
PrefHashFilter::OnWriteCallbackPair callbacks =
pref_hash_filter_->FilterSerializeData(&root_dict);
ASSERT_FALSE(callbacks.first.is_null());
// Prefs should be cleared from the external validation store only once the
// before-write callback is run.
ASSERT_EQ(
0u, mock_external_validation_hash_store_contents_->cleared_paths_count());
callbacks.first.Run();
ASSERT_EQ(
2u, mock_external_validation_hash_store_contents_->cleared_paths_count());
// No pref write should occur before the after-write callback is run.
ASSERT_EQ(
0u, mock_external_validation_hash_store_contents_->stored_hashes_count());
callbacks.second.Run(true);
ASSERT_EQ(
2u, mock_external_validation_hash_store_contents_->stored_hashes_count());
ASSERT_EQ(
"atomic mac for: atomic_pref",
mock_external_validation_hash_store_contents_->GetStoredMac(kAtomicPref));
ASSERT_EQ("split mac for: split_pref/a",
mock_external_validation_hash_store_contents_->GetStoredSplitMac(
kSplitPref, "a"));
// The callbacks should write directly to the contents without going through
// a pref hash store.
ASSERT_EQ(0u,
mock_external_validation_pref_hash_store_->stored_paths_count());
}
TEST_P(PrefHashFilterTest, CallFilterSerializeDataCallbacksWithFailure) {
base::DictionaryValue root_dict;
// Ownership of the following values is transfered to |root_dict|.
base::Value* int_value1 = new base::FundamentalValue(1);
root_dict.Set(kAtomicPref, int_value1);
// Only update kAtomicPref.
pref_hash_filter_->FilterUpdate(kAtomicPref);
PrefHashFilter::OnWriteCallbackPair callbacks =
pref_hash_filter_->FilterSerializeData(&root_dict);
ASSERT_FALSE(callbacks.first.is_null());
callbacks.first.Run();
// The pref should have been cleared from the external validation store.
ASSERT_EQ(
1u, mock_external_validation_hash_store_contents_->cleared_paths_count());
callbacks.second.Run(false);
// Expect no writes to the external validation hash store contents.
ASSERT_EQ(0u,
mock_external_validation_pref_hash_store_->stored_paths_count());
ASSERT_EQ(
0u, mock_external_validation_hash_store_contents_->stored_hashes_count());
}
TEST_P(PrefHashFilterTest, ExternalValidationValueChanged) {
// Ownership of this value is transfered to |pref_store_contents_|.
base::Value* int_value = new base::FundamentalValue(1234);
pref_store_contents_->Set(kAtomicPref, int_value);
base::DictionaryValue* dict_value = new base::DictionaryValue;
dict_value->SetString("a", "foo");
dict_value->SetInteger("b", 1234);
dict_value->SetInteger("c", 56);
dict_value->SetBoolean("d", false);
pref_store_contents_->Set(kSplitPref, dict_value);
mock_external_validation_pref_hash_store_->SetCheckResult(
kAtomicPref, PrefHashStoreTransaction::CHANGED);
mock_external_validation_pref_hash_store_->SetCheckResult(
kSplitPref, PrefHashStoreTransaction::CHANGED);
std::vector<std::string> mock_invalid_keys;
mock_invalid_keys.push_back("a");
mock_invalid_keys.push_back("c");
mock_external_validation_pref_hash_store_->SetInvalidKeysResult(
kSplitPref, mock_invalid_keys);
DoFilterOnLoad(false);
ASSERT_EQ(arraysize(kTestTrackedPrefs),
mock_external_validation_pref_hash_store_->checked_paths_count());
ASSERT_EQ(2u,
mock_external_validation_pref_hash_store_->stored_paths_count());
ASSERT_EQ(
1u, mock_external_validation_pref_hash_store_->transactions_performed());
// TODO(proberge): query mock_validation_state_ for number of CHANGED
// and UNCHANGED preferences once the class supports external validation.
}
INSTANTIATE_TEST_CASE_P(PrefHashFilterTestInstance, INSTANTIATE_TEST_CASE_P(PrefHashFilterTestInstance,
PrefHashFilterTest, PrefHashFilterTest,
testing::Values(PrefHashFilter::NO_ENFORCEMENT, testing::Values(PrefHashFilter::NO_ENFORCEMENT,
......
...@@ -12,7 +12,6 @@ ...@@ -12,7 +12,6 @@
#include "components/user_prefs/tracked/pref_hash_store_transaction.h" #include "components/user_prefs/tracked/pref_hash_store_transaction.h"
class HashStoreContents; class HashStoreContents;
class PrefHashStoreTransaction;
// Holds the configuration and implementation used to calculate and verify // Holds the configuration and implementation used to calculate and verify
// preference MACs. // preference MACs.
......
...@@ -7,7 +7,6 @@ ...@@ -7,7 +7,6 @@
#include <windows.h> #include <windows.h>
#include "base/logging.h" #include "base/logging.h"
#include "base/memory/ptr_util.h"
#include "base/numerics/safe_conversions.h" #include "base/numerics/safe_conversions.h"
#include "base/strings/string_split.h" #include "base/strings/string_split.h"
#include "base/strings/stringprintf.h" #include "base/strings/stringprintf.h"
...@@ -20,7 +19,7 @@ using base::win::RegistryValueIterator; ...@@ -20,7 +19,7 @@ using base::win::RegistryValueIterator;
namespace { namespace {
constexpr size_t kMacSize = 64; constexpr size_t kMacSize = 32;
base::string16 GetSplitPrefKeyName(const base::string16& reg_key_name, base::string16 GetSplitPrefKeyName(const base::string16& reg_key_name,
const std::string& split_key_name) { const std::string& split_key_name) {
...@@ -73,18 +72,6 @@ RegistryHashStoreContentsWin::RegistryHashStoreContentsWin( ...@@ -73,18 +72,6 @@ RegistryHashStoreContentsWin::RegistryHashStoreContentsWin(
const base::string16& store_key) const base::string16& store_key)
: preference_key_name_(registry_path + L"\\PreferenceMACs\\" + store_key) {} : preference_key_name_(registry_path + L"\\PreferenceMACs\\" + store_key) {}
RegistryHashStoreContentsWin::RegistryHashStoreContentsWin(
const RegistryHashStoreContentsWin& other) = default;
bool RegistryHashStoreContentsWin::IsCopyable() const {
return true;
}
std::unique_ptr<HashStoreContents> RegistryHashStoreContentsWin::MakeCopy()
const {
return base::WrapUnique(new RegistryHashStoreContentsWin(*this));
}
base::StringPiece RegistryHashStoreContentsWin::GetUMASuffix() const { base::StringPiece RegistryHashStoreContentsWin::GetUMASuffix() const {
return user_prefs::tracked::kTrackedPrefRegistryValidationSuffix; return user_prefs::tracked::kTrackedPrefRegistryValidationSuffix;
} }
...@@ -154,6 +141,7 @@ void RegistryHashStoreContentsWin::SetSplitMac(const std::string& path, ...@@ -154,6 +141,7 @@ void RegistryHashStoreContentsWin::SetSplitMac(const std::string& path,
} }
bool RegistryHashStoreContentsWin::RemoveEntry(const std::string& path) { bool RegistryHashStoreContentsWin::RemoveEntry(const std::string& path) {
// ClearSplitMac is first to avoid short-circuit issues.
return ClearAtomicMac(preference_key_name_, path) || return ClearAtomicMac(preference_key_name_, path) ||
ClearSplitMac(preference_key_name_, path); ClearSplitMac(preference_key_name_, path);
} }
...@@ -161,22 +149,22 @@ bool RegistryHashStoreContentsWin::RemoveEntry(const std::string& path) { ...@@ -161,22 +149,22 @@ bool RegistryHashStoreContentsWin::RemoveEntry(const std::string& path) {
void RegistryHashStoreContentsWin::ImportEntry(const std::string& path, void RegistryHashStoreContentsWin::ImportEntry(const std::string& path,
const base::Value* in_value) { const base::Value* in_value) {
NOTREACHED() NOTREACHED()
<< "RegistryHashStoreContents does not support the ImportEntry operation"; << "RegistryHashStore does not support the ImportEntry operation";
} }
const base::DictionaryValue* RegistryHashStoreContentsWin::GetContents() const { const base::DictionaryValue* RegistryHashStoreContentsWin::GetContents() const {
NOTREACHED() NOTREACHED()
<< "RegistryHashStoreContents does not support the GetContents operation"; << "RegistryHashStore does not support the GetContents operation";
return NULL; return NULL;
} }
std::string RegistryHashStoreContentsWin::GetSuperMac() const { std::string RegistryHashStoreContentsWin::GetSuperMac() const {
NOTREACHED() NOTREACHED()
<< "RegistryHashStoreContents does not support the GetSuperMac operation"; << "RegistryHashStore does not support the GetSuperMac operation";
return NULL; return NULL;
} }
void RegistryHashStoreContentsWin::SetSuperMac(const std::string& super_mac) { void RegistryHashStoreContentsWin::SetSuperMac(const std::string& super_mac) {
NOTREACHED() NOTREACHED()
<< "RegistryHashStoreContents does not support the SetSuperMac operation"; << "RegistryHashStore does not support the SetSuperMac operation";
} }
...@@ -18,8 +18,6 @@ class RegistryHashStoreContentsWin : public HashStoreContents { ...@@ -18,8 +18,6 @@ class RegistryHashStoreContentsWin : public HashStoreContents {
const base::string16& store_key); const base::string16& store_key);
// HashStoreContents overrides: // HashStoreContents overrides:
bool IsCopyable() const override;
std::unique_ptr<HashStoreContents> MakeCopy() const override;
base::StringPiece GetUMASuffix() const override; base::StringPiece GetUMASuffix() const override;
void Reset() override; void Reset() override;
bool GetMac(const std::string& path, std::string* out_value) override; bool GetMac(const std::string& path, std::string* out_value) override;
...@@ -39,11 +37,9 @@ class RegistryHashStoreContentsWin : public HashStoreContents { ...@@ -39,11 +37,9 @@ class RegistryHashStoreContentsWin : public HashStoreContents {
void SetSuperMac(const std::string& super_mac) override; void SetSuperMac(const std::string& super_mac) override;
private: private:
// Helper constructor for |MakeCopy|.
explicit RegistryHashStoreContentsWin(
const RegistryHashStoreContentsWin& other);
const base::string16 preference_key_name_; const base::string16 preference_key_name_;
DISALLOW_COPY_AND_ASSIGN(RegistryHashStoreContentsWin);
}; };
#endif // COMPONENTS_USER_PREFS_TRACKED_PREF_REGISTRY_HASH_STORE_CONTENTS_H_ #endif // COMPONENTS_USER_PREFS_TRACKED_PREF_REGISTRY_HASH_STORE_CONTENTS_H_
...@@ -16,11 +16,9 @@ namespace { ...@@ -16,11 +16,9 @@ namespace {
constexpr base::char16 kRegistryPath[] = L"Foo\\TestStore"; constexpr base::char16 kRegistryPath[] = L"Foo\\TestStore";
constexpr base::char16 kStoreKey[] = L"test_store_key"; constexpr base::char16 kStoreKey[] = L"test_store_key";
// Hex-encoded MACs are 64 characters long. // MACs are 32 characters long.
constexpr char kTestStringA[] = constexpr char kTestStringA[] = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; constexpr char kTestStringB[] = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb";
constexpr char kTestStringB[] =
"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb";
constexpr char kAtomicPrefPath[] = "path1"; constexpr char kAtomicPrefPath[] = "path1";
constexpr char kSplitPrefPath[] = "extension"; constexpr char kSplitPrefPath[] = "extension";
...@@ -116,4 +114,4 @@ TEST_F(RegistryHashStoreContentsWinTest, TestReset) { ...@@ -116,4 +114,4 @@ TEST_F(RegistryHashStoreContentsWinTest, TestReset) {
split_macs.clear(); split_macs.clear();
EXPECT_FALSE(contents->GetSplitMacs(kSplitPrefPath, &split_macs)); EXPECT_FALSE(contents->GetSplitMacs(kSplitPrefPath, &split_macs));
EXPECT_EQ(0U, split_macs.size()); EXPECT_EQ(0U, split_macs.size());
} }
\ No newline at end of file
...@@ -24,10 +24,6 @@ TrackedAtomicPreference::TrackedAtomicPreference( ...@@ -24,10 +24,6 @@ TrackedAtomicPreference::TrackedAtomicPreference(
delegate_(delegate) { delegate_(delegate) {
} }
TrackedPreferenceType TrackedAtomicPreference::GetType() const {
return TrackedPreferenceType::ATOMIC;
}
void TrackedAtomicPreference::OnNewValue( void TrackedAtomicPreference::OnNewValue(
const base::Value* value, const base::Value* value,
PrefHashStoreTransaction* transaction) const { PrefHashStoreTransaction* transaction) const {
...@@ -36,32 +32,20 @@ void TrackedAtomicPreference::OnNewValue( ...@@ -36,32 +32,20 @@ void TrackedAtomicPreference::OnNewValue(
bool TrackedAtomicPreference::EnforceAndReport( bool TrackedAtomicPreference::EnforceAndReport(
base::DictionaryValue* pref_store_contents, base::DictionaryValue* pref_store_contents,
PrefHashStoreTransaction* transaction, PrefHashStoreTransaction* transaction) const {
PrefHashStoreTransaction* external_validation_transaction) const {
const base::Value* value = NULL; const base::Value* value = NULL;
pref_store_contents->Get(pref_path_, &value); pref_store_contents->Get(pref_path_, &value);
PrefHashStoreTransaction::ValueState value_state = PrefHashStoreTransaction::ValueState value_state =
transaction->CheckValue(pref_path_, value); transaction->CheckValue(pref_path_, value);
helper_.ReportValidationResult(value_state, transaction->GetStoreUMASuffix());
PrefHashStoreTransaction::ValueState external_validation_value_state = helper_.ReportValidationResult(value_state, transaction->GetStoreUMASuffix());
PrefHashStoreTransaction::UNCHANGED;
if (external_validation_transaction) {
external_validation_value_state =
external_validation_transaction->CheckValue(pref_path_, value);
helper_.ReportValidationResult(
external_validation_value_state,
external_validation_transaction->GetStoreUMASuffix());
// TODO(proberge): Call delegate_->OnAtomicPreferenceValidation.
}
TrackedPreferenceHelper::ResetAction reset_action =
helper_.GetAction(value_state);
if (delegate_) { if (delegate_) {
delegate_->OnAtomicPreferenceValidation(pref_path_, value, value_state, delegate_->OnAtomicPreferenceValidation(pref_path_, value, value_state,
helper_.IsPersonal()); helper_.IsPersonal());
} }
TrackedPreferenceHelper::ResetAction reset_action =
helper_.GetAction(value_state);
helper_.ReportAction(reset_action); helper_.ReportAction(reset_action);
bool was_reset = false; bool was_reset = false;
...@@ -77,16 +61,5 @@ bool TrackedAtomicPreference::EnforceAndReport( ...@@ -77,16 +61,5 @@ bool TrackedAtomicPreference::EnforceAndReport(
transaction->StoreHash(pref_path_, new_value); transaction->StoreHash(pref_path_, new_value);
} }
// Update MACs in the external store if there is one and there either was a
// reset or external validation failed.
if (external_validation_transaction &&
(was_reset ||
external_validation_value_state !=
PrefHashStoreTransaction::UNCHANGED)) {
const base::Value* new_value = nullptr;
pref_store_contents->Get(pref_path_, &new_value);
external_validation_transaction->StoreHash(pref_path_, new_value);
}
return was_reset; return was_reset;
} }
...@@ -30,13 +30,10 @@ class TrackedAtomicPreference : public TrackedPreference { ...@@ -30,13 +30,10 @@ class TrackedAtomicPreference : public TrackedPreference {
TrackedPreferenceValidationDelegate* delegate); TrackedPreferenceValidationDelegate* delegate);
// TrackedPreference implementation. // TrackedPreference implementation.
TrackedPreferenceType GetType() const override;
void OnNewValue(const base::Value* value, void OnNewValue(const base::Value* value,
PrefHashStoreTransaction* transaction) const override; PrefHashStoreTransaction* transaction) const override;
bool EnforceAndReport( bool EnforceAndReport(base::DictionaryValue* pref_store_contents,
base::DictionaryValue* pref_store_contents, PrefHashStoreTransaction* transaction) const override;
PrefHashStoreTransaction* transaction,
PrefHashStoreTransaction* external_validation_transaction) const override;
private: private:
const std::string pref_path_; const std::string pref_path_;
......
...@@ -12,16 +12,12 @@ class DictionaryValue; ...@@ -12,16 +12,12 @@ class DictionaryValue;
class Value; class Value;
} }
enum class TrackedPreferenceType { ATOMIC, SPLIT };
// A TrackedPreference tracks changes to an individual preference, reporting and // A TrackedPreference tracks changes to an individual preference, reporting and
// reacting to them according to preference-specific and browser-wide policies. // reacting to them according to preference-specific and browser-wide policies.
class TrackedPreference { class TrackedPreference {
public: public:
virtual ~TrackedPreference() {} virtual ~TrackedPreference() {}
virtual TrackedPreferenceType GetType() const = 0;
// Notifies the underlying TrackedPreference about its new |value| which // Notifies the underlying TrackedPreference about its new |value| which
// can update hashes in the corresponding hash store via |transaction|. // can update hashes in the corresponding hash store via |transaction|.
virtual void OnNewValue(const base::Value* value, virtual void OnNewValue(const base::Value* value,
...@@ -31,14 +27,10 @@ class TrackedPreference { ...@@ -31,14 +27,10 @@ class TrackedPreference {
// is valid. Responds to verification failures according to // is valid. Responds to verification failures according to
// preference-specific and browser-wide policy and reports results to via UMA. // preference-specific and browser-wide policy and reports results to via UMA.
// May use |transaction| to check/modify hashes in the corresponding hash // May use |transaction| to check/modify hashes in the corresponding hash
// store. Performs validation and reports results without enforcing for // store.
// |external_validation_transaction|. This call assumes exclusive access to
// |external_validation_transaction| and its associated state and as such
// should only be called before any other subsystem is made aware of it.
virtual bool EnforceAndReport( virtual bool EnforceAndReport(
base::DictionaryValue* pref_store_contents, base::DictionaryValue* pref_store_contents,
PrefHashStoreTransaction* transaction, PrefHashStoreTransaction* transaction) const = 0;
PrefHashStoreTransaction* external_validation_transaction) const = 0;
}; };
#endif // COMPONENTS_USER_PREFS_TRACKED_TRACKED_PREFERENCE_H_ #endif // COMPONENTS_USER_PREFS_TRACKED_TRACKED_PREFERENCE_H_
...@@ -27,10 +27,6 @@ TrackedSplitPreference::TrackedSplitPreference( ...@@ -27,10 +27,6 @@ TrackedSplitPreference::TrackedSplitPreference(
delegate_(delegate) { delegate_(delegate) {
} }
TrackedPreferenceType TrackedSplitPreference::GetType() const {
return TrackedPreferenceType::SPLIT;
}
void TrackedSplitPreference::OnNewValue( void TrackedSplitPreference::OnNewValue(
const base::Value* value, const base::Value* value,
PrefHashStoreTransaction* transaction) const { PrefHashStoreTransaction* transaction) const {
...@@ -44,8 +40,7 @@ void TrackedSplitPreference::OnNewValue( ...@@ -44,8 +40,7 @@ void TrackedSplitPreference::OnNewValue(
bool TrackedSplitPreference::EnforceAndReport( bool TrackedSplitPreference::EnforceAndReport(
base::DictionaryValue* pref_store_contents, base::DictionaryValue* pref_store_contents,
PrefHashStoreTransaction* transaction, PrefHashStoreTransaction* transaction) const {
PrefHashStoreTransaction* external_validation_transaction) const {
base::DictionaryValue* dict_value = NULL; base::DictionaryValue* dict_value = NULL;
if (!pref_store_contents->GetDictionary(pref_path_, &dict_value) && if (!pref_store_contents->GetDictionary(pref_path_, &dict_value) &&
pref_store_contents->Get(pref_path_, NULL)) { pref_store_contents->Get(pref_path_, NULL)) {
...@@ -63,26 +58,12 @@ bool TrackedSplitPreference::EnforceAndReport( ...@@ -63,26 +58,12 @@ bool TrackedSplitPreference::EnforceAndReport(
helper_.ReportValidationResult(value_state, transaction->GetStoreUMASuffix()); helper_.ReportValidationResult(value_state, transaction->GetStoreUMASuffix());
PrefHashStoreTransaction::ValueState external_validation_value_state = TrackedPreferenceHelper::ResetAction reset_action =
PrefHashStoreTransaction::UNCHANGED; helper_.GetAction(value_state);
if (external_validation_transaction) {
std::vector<std::string> invalid_external_validation_keys;
external_validation_value_state =
external_validation_transaction->CheckSplitValue(
pref_path_, dict_value, &invalid_external_validation_keys);
helper_.ReportValidationResult(
external_validation_value_state,
external_validation_transaction->GetStoreUMASuffix());
// TODO(proberge): Call delegate_->OnSplitPreferenceValidation.
}
if (delegate_) { if (delegate_) {
delegate_->OnSplitPreferenceValidation(pref_path_, dict_value, invalid_keys, delegate_->OnSplitPreferenceValidation(pref_path_, dict_value, invalid_keys,
value_state, helper_.IsPersonal()); value_state, helper_.IsPersonal());
} }
TrackedPreferenceHelper::ResetAction reset_action =
helper_.GetAction(value_state);
helper_.ReportAction(reset_action); helper_.ReportAction(reset_action);
bool was_reset = false; bool was_reset = false;
...@@ -107,16 +88,5 @@ bool TrackedSplitPreference::EnforceAndReport( ...@@ -107,16 +88,5 @@ bool TrackedSplitPreference::EnforceAndReport(
transaction->StoreSplitHash(pref_path_, new_dict_value); transaction->StoreSplitHash(pref_path_, new_dict_value);
} }
// Update MACs in the external store if there is one and there either was a
// reset or external validation failed.
if (external_validation_transaction &&
(was_reset ||
external_validation_value_state !=
PrefHashStoreTransaction::UNCHANGED)) {
const base::DictionaryValue* new_dict_value = nullptr;
pref_store_contents->GetDictionary(pref_path_, &new_dict_value);
external_validation_transaction->StoreSplitHash(pref_path_, new_dict_value);
}
return was_reset; return was_reset;
} }
...@@ -33,13 +33,10 @@ class TrackedSplitPreference : public TrackedPreference { ...@@ -33,13 +33,10 @@ class TrackedSplitPreference : public TrackedPreference {
TrackedPreferenceValidationDelegate* delegate); TrackedPreferenceValidationDelegate* delegate);
// TrackedPreference implementation. // TrackedPreference implementation.
TrackedPreferenceType GetType() const override;
void OnNewValue(const base::Value* value, void OnNewValue(const base::Value* value,
PrefHashStoreTransaction* transaction) const override; PrefHashStoreTransaction* transaction) const override;
bool EnforceAndReport( bool EnforceAndReport(base::DictionaryValue* pref_store_contents,
base::DictionaryValue* pref_store_contents, PrefHashStoreTransaction* transaction) const override;
PrefHashStoreTransaction* transaction,
PrefHashStoreTransaction* external_validation_transaction) const override;
private: private:
const std::string pref_path_; const std::string pref_path_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment