From 10458eca117abc6737a2efcd99f58e6a0b72437e Mon Sep 17 00:00:00 2001
From: Tej Singh <singhtejinder@google.com>
Date: Tue, 17 Mar 2020 11:04:02 -0700
Subject: Enforce permission on native puller API

Test: m
Test: no security exceptions on boot
Test: atest LibStatsPullTests
Bug: 148955001
Change-Id: I4b06bfc41be2925270eaddd717f1499d98739dae
---
 apex/statsd/aidl/android/os/IStatsd.aidl           | 14 +++++++++-----
 apex/statsd/tests/libstatspull/AndroidManifest.xml |  1 +
 cmds/statsd/src/StatsService.cpp                   | 17 +++++++++++++++--
 data/etc/platform.xml                              |  2 ++
 4 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/apex/statsd/aidl/android/os/IStatsd.aidl b/apex/statsd/aidl/android/os/IStatsd.aidl
index c8aec5337f6e..445ae1d7a13e 100644
--- a/apex/statsd/aidl/android/os/IStatsd.aidl
+++ b/apex/statsd/aidl/android/os/IStatsd.aidl
@@ -190,10 +190,12 @@ interface IStatsd {
                                          long timeoutMillis,in int[] additiveFields,
                                          IPullAtomCallback pullerCallback);
 
-   /**
-    * Registers a puller callback function that, when invoked, pulls the data
-    * for the specified atom tag.
-    */
+    /**
+     * Registers a puller callback function that, when invoked, pulls the data
+     * for the specified atom tag.
+     *
+     * Enforces the REGISTER_STATS_PULL_ATOM permission.
+     */
     oneway void registerNativePullAtomCallback(int atomTag, long coolDownNs, long timeoutNs,
                            in int[] additiveFields, IPullAtomCallback pullerCallback);
 
@@ -203,7 +205,9 @@ interface IStatsd {
     oneway void unregisterPullAtomCallback(int uid, int atomTag);
 
     /**
-     * Unregisters any pullAtomCallback for the given atom.
+     * Unregisters any pullAtomCallback for the given atom + caller.
+     *
+     * Enforces the REGISTER_STATS_PULL_ATOM permission.
      */
     oneway void unregisterNativePullAtomCallback(int atomTag);
 
diff --git a/apex/statsd/tests/libstatspull/AndroidManifest.xml b/apex/statsd/tests/libstatspull/AndroidManifest.xml
index bffd400bdb2c..0c669b051c86 100644
--- a/apex/statsd/tests/libstatspull/AndroidManifest.xml
+++ b/apex/statsd/tests/libstatspull/AndroidManifest.xml
@@ -21,6 +21,7 @@
 
     <uses-permission android:name="android.permission.DUMP" />
     <uses-permission android:name="android.permission.PACKAGE_USAGE_STATS" />
+    <uses-permission android:name="android.permission.REGISTER_STATS_PULL_ATOM" />
 
     <instrumentation android:name="androidx.test.runner.AndroidJUnitRunner"
                      android:targetPackage="com.android.internal.os.statsd.libstats"
diff --git a/cmds/statsd/src/StatsService.cpp b/cmds/statsd/src/StatsService.cpp
index 07579bb21860..7ad982671860 100644
--- a/cmds/statsd/src/StatsService.cpp
+++ b/cmds/statsd/src/StatsService.cpp
@@ -53,6 +53,8 @@ namespace statsd {
 
 constexpr const char* kPermissionDump = "android.permission.DUMP";
 
+constexpr const char* kPermissionRegisterPullAtom = "android.permission.REGISTER_STATS_PULL_ATOM";
+
 #define STATS_SERVICE_DIR "/data/misc/stats-service"
 
 // for StatsDataDumpProto
@@ -60,7 +62,7 @@ const int FIELD_ID_REPORTS_LIST = 1;
 
 static Status exception(int32_t code, const std::string& msg) {
     ALOGE("%s (%d)", msg.c_str(), code);
-    return ::ndk::ScopedAStatus(AStatus_fromExceptionCodeWithMessage(code, msg.c_str()));
+    return Status::fromExceptionCodeWithMessage(code, msg.c_str());
 }
 
 static bool checkPermission(const char* permission) {
@@ -1210,7 +1212,12 @@ Status StatsService::registerPullAtomCallback(int32_t uid, int32_t atomTag, int6
 Status StatsService::registerNativePullAtomCallback(int32_t atomTag, int64_t coolDownNs,
                                     int64_t timeoutNs, const std::vector<int32_t>& additiveFields,
                                     const shared_ptr<IPullAtomCallback>& pullerCallback) {
-
+    if (!checkPermission(kPermissionRegisterPullAtom)) {
+        return exception(
+                EX_SECURITY,
+                StringPrintf("Uid %d does not have the %s permission when registering atom %d",
+                             AIBinder_getCallingUid(), kPermissionRegisterPullAtom, atomTag));
+    }
     VLOG("StatsService::registerNativePullAtomCallback called.");
     int32_t uid = AIBinder_getCallingUid();
     mPullerManager->RegisterPullAtomCallback(uid, atomTag, coolDownNs, timeoutNs, additiveFields,
@@ -1226,6 +1233,12 @@ Status StatsService::unregisterPullAtomCallback(int32_t uid, int32_t atomTag) {
 }
 
 Status StatsService::unregisterNativePullAtomCallback(int32_t atomTag) {
+    if (!checkPermission(kPermissionRegisterPullAtom)) {
+        return exception(
+                EX_SECURITY,
+                StringPrintf("Uid %d does not have the %s permission when unregistering atom %d",
+                             AIBinder_getCallingUid(), kPermissionRegisterPullAtom, atomTag));
+    }
     VLOG("StatsService::unregisterNativePullAtomCallback called.");
     int32_t uid = AIBinder_getCallingUid();
     mPullerManager->UnregisterPullAtomCallback(uid, atomTag);
diff --git a/data/etc/platform.xml b/data/etc/platform.xml
index 6929d0d1879d..f63ec6bd04c3 100644
--- a/data/etc/platform.xml
+++ b/data/etc/platform.xml
@@ -190,6 +190,8 @@
     <assign-permission name="android.permission.STATSCOMPANION" uid="statsd" />
     <assign-permission name="android.permission.UPDATE_APP_OPS_STATS" uid="statsd" />
 
+    <assign-permission name="android.permission.REGISTER_STATS_PULL_ATOM" uid="gpu_service" />
+
     <split-permission name="android.permission.ACCESS_FINE_LOCATION">
         <new-permission name="android.permission.ACCESS_COARSE_LOCATION" />
     </split-permission>
-- 
cgit v1.2.3-59-g8ed1b