From b1eee0beab855a35bba74b2b0f8c5152e5664069 Mon Sep 17 00:00:00 2001 From: Muhammad Qureshi Date: Fri, 15 May 2020 05:46:38 -0700 Subject: [PATCH] Don't create StateTrackers for whitelisted atoms Whitelisted atoms can be logged from any app so integrity of StateTrackers for those atoms cannot be guaranteed. Fixes: 155522551 Test: statsd_test Change-Id: Ifc64e9168ee5f1f8977b99534319bc65946630f0 --- cmds/statsd/src/metrics/MetricProducer.h | 2 +- .../src/metrics/metrics_manager_util.cpp | 11 ++++++- cmds/statsd/tests/MetricsManager_test.cpp | 29 +++++++++++++++++++ 3 files changed, 40 insertions(+), 2 deletions(-) diff --git a/cmds/statsd/src/metrics/MetricProducer.h b/cmds/statsd/src/metrics/MetricProducer.h index 91c98ea27269b..28563ad4b0f50 100644 --- a/cmds/statsd/src/metrics/MetricProducer.h +++ b/cmds/statsd/src/metrics/MetricProducer.h @@ -442,7 +442,7 @@ protected: bool mIsActive; // The slice_by_state atom ids defined in statsd_config. - std::vector mSlicedStateAtoms; + const std::vector mSlicedStateAtoms; // Maps atom ids and state values to group_ids (>). const std::unordered_map> mStateGroupMap; diff --git a/cmds/statsd/src/metrics/metrics_manager_util.cpp b/cmds/statsd/src/metrics/metrics_manager_util.cpp index 3ab44f4a06af8..210d382b1363f 100644 --- a/cmds/statsd/src/metrics/metrics_manager_util.cpp +++ b/cmds/statsd/src/metrics/metrics_manager_util.cpp @@ -791,10 +791,19 @@ bool initMetrics(const ConfigKey& key, const StatsdConfig& config, const int64_t } noReportMetricIds.insert(no_report_metric); } + + const set whitelistedAtomIds(config.whitelisted_atom_ids().begin(), + config.whitelisted_atom_ids().end()); for (const auto& it : allMetricProducers) { // Register metrics to StateTrackers for (int atomId : it->getSlicedStateAtoms()) { - StateManager::getInstance().registerListener(atomId, it); + // Register listener for non-whitelisted atoms only. Using whitelisted atom as a sliced + // state atom is not allowed. + if (whitelistedAtomIds.find(atomId) == whitelistedAtomIds.end()) { + StateManager::getInstance().registerListener(atomId, it); + } else { + return false; + } } } return true; diff --git a/cmds/statsd/tests/MetricsManager_test.cpp b/cmds/statsd/tests/MetricsManager_test.cpp index 15be0d2be0488..ef8a34e25d730 100644 --- a/cmds/statsd/tests/MetricsManager_test.cpp +++ b/cmds/statsd/tests/MetricsManager_test.cpp @@ -29,6 +29,7 @@ #include "src/metrics/MetricProducer.h" #include "src/metrics/ValueMetricProducer.h" #include "src/metrics/metrics_manager_util.h" +#include "src/state/StateManager.h" #include "statsd_test_util.h" using namespace testing; @@ -615,6 +616,34 @@ TEST(MetricsManagerTest, TestCheckLogCredentialsWhitelistedAtom) { EXPECT_TRUE(metricsManager.checkLogCredentials(event)); } +TEST(MetricsManagerTest, TestWhitelistedAtomStateTracker) { + sp uidMap; + sp pullerManager = new StatsPullerManager(); + sp anomalyAlarmMonitor; + sp periodicAlarmMonitor; + + StatsdConfig config = buildGoodConfig(); + config.add_allowed_log_source("AID_SYSTEM"); + config.add_whitelisted_atom_ids(3); + config.add_whitelisted_atom_ids(4); + + State state; + state.set_id(1); + state.set_atom_id(3); + + *config.add_state() = state; + + config.mutable_count_metric(0)->add_slice_by_state(state.id()); + + StateManager::getInstance().clear(); + + MetricsManager metricsManager(kConfigKey, config, timeBaseSec, timeBaseSec, uidMap, + pullerManager, anomalyAlarmMonitor, periodicAlarmMonitor); + + EXPECT_EQ(0, StateManager::getInstance().getStateTrackersCount()); + EXPECT_FALSE(metricsManager.isConfigValid()); +} + } // namespace statsd } // namespace os } // namespace android