MediaMetrics: Use AllowUid property for services to control client access
Remove implicit first client uid access.
Test: adb shell dumpsys media.metrics
Bug: 149850236
Change-Id: I4f953db4688bf5f174c3e1611f492f31d2926e96
diff --git a/services/audioflinger/Tracks.cpp b/services/audioflinger/Tracks.cpp
index 23d8329..4898d37 100644
--- a/services/audioflinger/Tracks.cpp
+++ b/services/audioflinger/Tracks.cpp
@@ -605,6 +605,7 @@
mediametrics::LogItem(mMetricsId)
.setPid(creatorPid)
.setUid(uid)
+ .set(AMEDIAMETRICS_PROP_ALLOWUID, (int32_t)uid)
.set(AMEDIAMETRICS_PROP_EVENT,
AMEDIAMETRICS_PROP_PREFIX_SERVER AMEDIAMETRICS_PROP_EVENT_VALUE_CTOR)
.record();
@@ -2165,6 +2166,7 @@
mediametrics::LogItem(mMetricsId)
.setPid(creatorPid)
.setUid(uid)
+ .set(AMEDIAMETRICS_PROP_ALLOWUID, (int32_t)uid)
.set(AMEDIAMETRICS_PROP_EVENT, "server." AMEDIAMETRICS_PROP_EVENT_VALUE_CTOR)
.record();
}
diff --git a/services/mediametrics/TimeMachine.h b/services/mediametrics/TimeMachine.h
index 6861c78..c82778b 100644
--- a/services/mediametrics/TimeMachine.h
+++ b/services/mediametrics/TimeMachine.h
@@ -75,21 +75,30 @@
class KeyHistory {
public:
template <typename T>
- KeyHistory(T key, pid_t pid, uid_t uid, int64_t time)
+ KeyHistory(T key, uid_t allowUid, int64_t time)
: mKey(key)
- , mPid(pid)
- , mUid(uid)
+ , mAllowUid(allowUid)
, mCreationTime(time)
, mLastModificationTime(time)
{
- putValue(BUNDLE_PID, (int32_t)pid, time);
- putValue(BUNDLE_UID, (int32_t)uid, time);
+ // allowUid allows an untrusted client with a matching uid to set properties
+ // in this key.
+ // If allowUid == (uid_t)-1, no untrusted client may set properties in the key.
+ if (allowUid != (uid_t)-1) {
+ // Set ALLOWUID property here; does not change after key creation.
+ putValue(AMEDIAMETRICS_PROP_ALLOWUID, (int32_t)allowUid, time);
+ }
}
KeyHistory(const KeyHistory &other) = default;
+ // Return NO_ERROR only if the passed in uidCheck is -1 or matches
+ // the internal mAllowUid.
+ // An external submit will always have a valid uidCheck parameter.
+ // An internal get request within mediametrics will have a uidCheck == -1 which
+ // we allow to proceed.
status_t checkPermission(uid_t uidCheck) const {
- return uidCheck != (uid_t)-1 && uidCheck != mUid ? PERMISSION_DENIED : NO_ERROR;
+ return uidCheck != (uid_t)-1 && uidCheck != mAllowUid ? PERMISSION_DENIED : NO_ERROR;
}
template <typename T>
@@ -199,8 +208,7 @@
}
const std::string mKey;
- const pid_t mPid __unused;
- const uid_t mUid;
+ const uid_t mAllowUid;
const int64_t mCreationTime __unused;
int64_t mLastModificationTime;
@@ -276,10 +284,13 @@
(void)gc(garbage);
+ // We set the allowUid for client access on key creation.
+ int32_t allowUid = -1;
+ (void)item->get(AMEDIAMETRICS_PROP_ALLOWUID, &allowUid);
// no keylock needed here as we are sole owner
// until placed on mHistory.
keyHistory = std::make_shared<KeyHistory>(
- key, item->getPid(), item->getUid(), time);
+ key, allowUid, time);
mHistory[key] = keyHistory;
} else {
keyHistory = it->second;
diff --git a/services/mediametrics/tests/mediametrics_tests.cpp b/services/mediametrics/tests/mediametrics_tests.cpp
index 78eb71c..cf0dceb 100644
--- a/services/mediametrics/tests/mediametrics_tests.cpp
+++ b/services/mediametrics/tests/mediametrics_tests.cpp
@@ -813,6 +813,42 @@
ASSERT_LT(9, audioAnalytics.dump(1000).second /* lines */);
}
+TEST(mediametrics_tests, audio_analytics_permission2) {
+ constexpr int32_t transactionUid = 1010; // arbitrary
+ auto item = std::make_shared<mediametrics::Item>("audio.1");
+ (*item).set("one", (int32_t)1)
+ .set("two", (int32_t)2)
+ .set(AMEDIAMETRICS_PROP_ALLOWUID, transactionUid)
+ .setTimestamp(10);
+
+ // item2 submitted untrusted
+ auto item2 = std::make_shared<mediametrics::Item>("audio.1");
+ (*item2).set("three", (int32_t)3)
+ .setUid(transactionUid)
+ .setTimestamp(11);
+
+ auto item3 = std::make_shared<mediametrics::Item>("audio.2");
+ (*item3).set("four", (int32_t)4)
+ .setTimestamp(12);
+
+ android::mediametrics::AudioAnalytics audioAnalytics;
+
+ // untrusted entities cannot create a new key.
+ ASSERT_EQ(PERMISSION_DENIED, audioAnalytics.submit(item, false /* isTrusted */));
+ ASSERT_EQ(PERMISSION_DENIED, audioAnalytics.submit(item2, false /* isTrusted */));
+
+ // TODO: Verify contents of AudioAnalytics.
+ // Currently there is no getter API in AudioAnalytics besides dump.
+ ASSERT_EQ(9, audioAnalytics.dump(1000).second /* lines */);
+
+ ASSERT_EQ(NO_ERROR, audioAnalytics.submit(item, true /* isTrusted */));
+ // untrusted entities can add to an existing key
+ ASSERT_EQ(NO_ERROR, audioAnalytics.submit(item2, false /* isTrusted */));
+
+ // Check that we have some info in the dump.
+ ASSERT_LT(9, audioAnalytics.dump(1000).second /* lines */);
+}
+
TEST(mediametrics_tests, audio_analytics_dump) {
auto item = std::make_shared<mediametrics::Item>("audio.1");
(*item).set("one", (int32_t)1)
diff --git a/services/oboeservice/AAudioServiceStreamBase.cpp b/services/oboeservice/AAudioServiceStreamBase.cpp
index 044c361..dba9fb9 100644
--- a/services/oboeservice/AAudioServiceStreamBase.cpp
+++ b/services/oboeservice/AAudioServiceStreamBase.cpp
@@ -105,6 +105,7 @@
mediametrics::LogItem(mMetricsId)
.setPid(getOwnerProcessId())
.setUid(getOwnerUserId())
+ .set(AMEDIAMETRICS_PROP_ALLOWUID, (int32_t)getOwnerUserId())
.set(AMEDIAMETRICS_PROP_EVENT, AMEDIAMETRICS_PROP_EVENT_VALUE_OPEN)
// the following are immutable
.set(AMEDIAMETRICS_PROP_BUFFERCAPACITYFRAMES, (int32_t)getBufferCapacity())