Snap for 5934975 from 0d0b69a0bbf235dc32ea070c98e9c301ee852a27 to sdk-release

Change-Id: I3c1194c078f551ea5628b7479f8c20a650819c90
diff --git a/libnetdbpf/BpfNetworkStats.cpp b/libnetdbpf/BpfNetworkStats.cpp
index e93a1e6..30b43fa 100644
--- a/libnetdbpf/BpfNetworkStats.cpp
+++ b/libnetdbpf/BpfNetworkStats.cpp
@@ -191,7 +191,7 @@
         return ret;
     }
 
-    BpfMap<uint32_t, uint8_t> configurationMap(mapRetrieve(CONFIGURATION_MAP_PATH, 0));
+    BpfMap<uint32_t, uint8_t> configurationMap(mapRetrieve(CONFIGURATION_MAP_PATH, BPF_OPEN_FLAGS));
     if (!configurationMap.isValid()) {
         int ret = -errno;
         ALOGE("get configuration map fd failed: %s", strerror(errno));
@@ -241,7 +241,10 @@
             return netdutils::status::ok;
         }
         StatsKey fakeKey = {
-            .uid = (uint32_t)UID_ALL, .counterSet = (uint32_t)SET_ALL, .tag = (uint32_t)TAG_NONE};
+                .uid = (uint32_t)UID_ALL,
+                .tag = (uint32_t)TAG_NONE,
+                .counterSet = (uint32_t)SET_ALL,
+        };
         lines->push_back(populateStatsEntry(fakeKey, value, ifname));
         return netdutils::status::ok;
     };
diff --git a/libnetdbpf/BpfNetworkStatsTest.cpp b/libnetdbpf/BpfNetworkStatsTest.cpp
index 8153127..057f2a9 100644
--- a/libnetdbpf/BpfNetworkStatsTest.cpp
+++ b/libnetdbpf/BpfNetworkStatsTest.cpp
@@ -201,10 +201,10 @@
     SKIP_IF_BPF_NOT_SUPPORTED;
 
     StatsValue value1 = {
-            .rxBytes = 0,
             .rxPackets = 0,
-            .txBytes = 0,
+            .rxBytes = 0,
             .txPackets = 0,
+            .txBytes = 0,
     };
     Stats result1 = {};
     ASSERT_EQ(0, bpfGetUidStatsInternal(TEST_UID1, &result1, mFakeAppUidStatsMap));
@@ -217,15 +217,17 @@
     updateIfaceMap(IFACE_NAME1, IFACE_INDEX1);
     updateIfaceMap(IFACE_NAME2, IFACE_INDEX2);
     updateIfaceMap(IFACE_NAME3, IFACE_INDEX3);
-    StatsValue value1 = {.rxBytes = TEST_BYTES0,
-                         .rxPackets = TEST_PACKET0,
-                         .txBytes = TEST_BYTES1,
-                         .txPackets = TEST_PACKET1,};
+    StatsValue value1 = {
+            .rxPackets = TEST_PACKET0,
+            .rxBytes = TEST_BYTES0,
+            .txPackets = TEST_PACKET1,
+            .txBytes = TEST_BYTES1,
+    };
     StatsValue value2 = {
-        .rxBytes = TEST_BYTES0 * 2,
-        .rxPackets = TEST_PACKET0 * 2,
-        .txBytes = TEST_BYTES1 * 2,
-        .txPackets = TEST_PACKET1 * 2,
+            .rxPackets = TEST_PACKET0 * 2,
+            .rxBytes = TEST_BYTES0 * 2,
+            .txPackets = TEST_PACKET1 * 2,
+            .txBytes = TEST_BYTES1 * 2,
     };
     ASSERT_TRUE(isOk(mFakeAppUidStatsMap.writeValue(TEST_UID1, value1, BPF_ANY)));
     ASSERT_TRUE(isOk(mFakeAppUidStatsMap.writeValue(TEST_UID2, value2, BPF_ANY)));
@@ -258,16 +260,16 @@
     updateIfaceMap(IFACE_NAME2, IFACE_INDEX2);
     updateIfaceMap(IFACE_NAME3, IFACE_INDEX3);
     StatsValue value1 = {
-        .rxBytes = TEST_BYTES0,
-        .rxPackets = TEST_PACKET0,
-        .txBytes = TEST_BYTES1,
-        .txPackets = TEST_PACKET1,
+            .rxPackets = TEST_PACKET0,
+            .rxBytes = TEST_BYTES0,
+            .txPackets = TEST_PACKET1,
+            .txBytes = TEST_BYTES1,
     };
     StatsValue value2 = {
-        .rxBytes = TEST_BYTES1,
-        .rxPackets = TEST_PACKET1,
-        .txBytes = TEST_BYTES0,
-        .txPackets = TEST_PACKET0,
+            .rxPackets = TEST_PACKET1,
+            .rxBytes = TEST_BYTES1,
+            .txPackets = TEST_PACKET0,
+            .txBytes = TEST_BYTES0,
     };
     uint32_t ifaceStatsKey = IFACE_INDEX1;
     EXPECT_TRUE(isOk(mFakeIfaceStatsMap.writeValue(ifaceStatsKey, value1, BPF_ANY)));
@@ -288,10 +290,10 @@
     ASSERT_EQ(0, bpfGetIfaceStatsInternal(NULL, &totalResult, mFakeIfaceStatsMap,
                                           mFakeIfaceIndexNameMap));
     StatsValue totalValue = {
-        .rxBytes = TEST_BYTES0 * 2 + TEST_BYTES1,
-        .rxPackets = TEST_PACKET0 * 2 + TEST_PACKET1,
-        .txBytes = TEST_BYTES1 * 2 + TEST_BYTES0,
-        .txPackets = TEST_PACKET1 * 2 + TEST_PACKET0,
+            .rxPackets = TEST_PACKET0 * 2 + TEST_PACKET1,
+            .rxBytes = TEST_BYTES0 * 2 + TEST_BYTES1,
+            .txPackets = TEST_PACKET1 * 2 + TEST_PACKET0,
+            .txBytes = TEST_BYTES1 * 2 + TEST_BYTES0,
     };
     expectStatsEqual(totalValue, totalResult);
 }
@@ -301,10 +303,12 @@
 
     updateIfaceMap(IFACE_NAME1, IFACE_INDEX1);
     updateIfaceMap(IFACE_NAME2, IFACE_INDEX2);
-    StatsValue value1 = {.rxBytes = TEST_BYTES0,
-                         .rxPackets = TEST_PACKET0,
-                         .txBytes = TEST_BYTES1,
-                         .txPackets = TEST_PACKET1,};
+    StatsValue value1 = {
+            .rxPackets = TEST_PACKET0,
+            .rxBytes = TEST_BYTES0,
+            .txPackets = TEST_PACKET1,
+            .txBytes = TEST_BYTES1,
+    };
     populateFakeStats(TEST_UID1, TEST_TAG, IFACE_INDEX1, TEST_COUNTERSET0, value1, mFakeStatsMap);
     populateFakeStats(TEST_UID1, TEST_TAG, IFACE_INDEX2, TEST_COUNTERSET0, value1, mFakeStatsMap);
     populateFakeStats(TEST_UID1, TEST_TAG + 1, IFACE_INDEX1, TEST_COUNTERSET0, value1,
@@ -336,10 +340,12 @@
 
     updateIfaceMap(IFACE_NAME1, IFACE_INDEX1);
     updateIfaceMap(IFACE_NAME2, IFACE_INDEX2);
-    StatsValue value1 = {.rxBytes = TEST_BYTES0,
-                         .rxPackets = TEST_PACKET0,
-                         .txBytes = TEST_BYTES1,
-                         .txPackets = TEST_PACKET1,};
+    StatsValue value1 = {
+            .rxPackets = TEST_PACKET0,
+            .rxBytes = TEST_BYTES0,
+            .txPackets = TEST_PACKET1,
+            .txBytes = TEST_BYTES1,
+    };
     populateFakeStats(0, 0, 0, OVERFLOW_COUNTERSET, value1, mFakeStatsMap);
     populateFakeStats(TEST_UID1, 0, IFACE_INDEX1, TEST_COUNTERSET0, value1, mFakeStatsMap);
     populateFakeStats(TEST_UID1, 0, IFACE_INDEX2, TEST_COUNTERSET0, value1, mFakeStatsMap);
@@ -370,22 +376,28 @@
     SKIP_IF_BPF_NOT_SUPPORTED;
 
     updateIfaceMap(IFACE_NAME1, IFACE_INDEX1);
-    StatsValue value1 = {.rxBytes = TEST_BYTES0 * 20,
-                         .rxPackets = TEST_PACKET0,
-                         .txBytes = TEST_BYTES1 * 20,
-                         .txPackets = TEST_PACKET1,};
+    StatsValue value1 = {
+            .rxPackets = TEST_PACKET0,
+            .rxBytes = TEST_BYTES0 * 20,
+            .txPackets = TEST_PACKET1,
+            .txBytes = TEST_BYTES1 * 20,
+    };
     uint32_t ifaceIndex = UNKNOWN_IFACE;
     populateFakeStats(TEST_UID1, 0, ifaceIndex, TEST_COUNTERSET0, value1, mFakeStatsMap);
     populateFakeStats(TEST_UID1, 0, IFACE_INDEX1, TEST_COUNTERSET0, value1, mFakeStatsMap);
-    StatsValue value2 = {.rxBytes = TEST_BYTES0 * 40,
-                         .rxPackets = TEST_PACKET0,
-                         .txBytes = TEST_BYTES1 * 40,
-                         .txPackets = TEST_PACKET1,};
+    StatsValue value2 = {
+            .rxPackets = TEST_PACKET0,
+            .rxBytes = TEST_BYTES0 * 40,
+            .txPackets = TEST_PACKET1,
+            .txBytes = TEST_BYTES1 * 40,
+    };
     populateFakeStats(TEST_UID1, 0, IFACE_INDEX2, TEST_COUNTERSET0, value2, mFakeStatsMap);
-    StatsKey curKey = {.uid = TEST_UID1,
-                       .tag = 0,
-                       .ifaceIndex = ifaceIndex,
-                       .counterSet = TEST_COUNTERSET0};
+    StatsKey curKey = {
+            .uid = TEST_UID1,
+            .tag = 0,
+            .counterSet = TEST_COUNTERSET0,
+            .ifaceIndex = ifaceIndex,
+    };
     char ifname[IFNAMSIZ];
     int64_t unknownIfaceBytesTotal = 0;
     ASSERT_EQ(-ENODEV, getIfaceNameFromMap(mFakeIfaceIndexNameMap, mFakeStatsMap, ifaceIndex,
@@ -412,16 +424,16 @@
     updateIfaceMap(IFACE_NAME3, IFACE_INDEX3);
     updateIfaceMap(LONG_IFACE_NAME, IFACE_INDEX4);
     StatsValue value1 = {
-        .rxBytes = TEST_BYTES0,
-        .rxPackets = TEST_PACKET0,
-        .txBytes = TEST_BYTES1,
-        .txPackets = TEST_PACKET1,
+            .rxPackets = TEST_PACKET0,
+            .rxBytes = TEST_BYTES0,
+            .txPackets = TEST_PACKET1,
+            .txBytes = TEST_BYTES1,
     };
     StatsValue value2 = {
-        .rxBytes = TEST_BYTES1,
-        .rxPackets = TEST_PACKET1,
-        .txBytes = TEST_BYTES0,
-        .txPackets = TEST_PACKET0,
+            .rxPackets = TEST_PACKET1,
+            .rxBytes = TEST_BYTES1,
+            .txPackets = TEST_PACKET0,
+            .txBytes = TEST_BYTES0,
     };
     uint32_t ifaceStatsKey = IFACE_INDEX1;
     EXPECT_TRUE(isOk(mFakeIfaceStatsMap.writeValue(ifaceStatsKey, value1, BPF_ANY)));
@@ -452,22 +464,22 @@
     updateIfaceMap(IFACE_NAME1, IFACE_INDEX3);  // Duplicate!
 
     StatsValue value1 = {
-            .rxBytes = TEST_BYTES0,
             .rxPackets = TEST_PACKET0,
-            .txBytes = TEST_BYTES1,
+            .rxBytes = TEST_BYTES0,
             .txPackets = TEST_PACKET1,
+            .txBytes = TEST_BYTES1,
     };
     StatsValue value2 = {
-            .rxBytes = TEST_BYTES1,
             .rxPackets = TEST_PACKET1,
-            .txBytes = TEST_BYTES0,
+            .rxBytes = TEST_BYTES1,
             .txPackets = TEST_PACKET0,
+            .txBytes = TEST_BYTES0,
     };
     StatsValue value3 = {
-            .rxBytes = TEST_BYTES0 * 2,
             .rxPackets = TEST_PACKET0 * 2,
-            .txBytes = TEST_BYTES1 * 2,
+            .rxBytes = TEST_BYTES0 * 2,
             .txPackets = TEST_PACKET1 * 2,
+            .txBytes = TEST_BYTES1 * 2,
     };
 
     std::vector<stats_line> lines;
@@ -541,10 +553,10 @@
     updateIfaceMap(IFACE_NAME1, IFACE_INDEX1);
 
     StatsValue value1 = {
-            .rxBytes = TEST_BYTES0,
             .rxPackets = TEST_PACKET0,
-            .txBytes = TEST_BYTES1,
+            .rxBytes = TEST_BYTES0,
             .txPackets = TEST_PACKET1,
+            .txBytes = TEST_BYTES1,
     };
 
     // Mutate uid, 0 < TEST_UID1 < INT_MAX < INT_MIN < UINT_MAX.
diff --git a/server/TrafficController.cpp b/server/TrafficController.cpp
index 4c55492..951226b 100644
--- a/server/TrafficController.cpp
+++ b/server/TrafficController.cpp
@@ -77,6 +77,7 @@
 // Otherwise, apps would be able to avoid data usage accounting entirely by filling up the
 // map with tagged traffic entries.
 constexpr int TOTAL_UID_STATS_ENTRIES_LIMIT = STATS_MAP_SIZE * 0.9;
+constexpr mode_t S_NONE = 0;
 
 static_assert(BPF_PERMISSION_INTERNET == INetd::PERMISSION_INTERNET,
               "Mismatch between BPF and AIDL permissions: PERMISSION_INTERNET");
@@ -166,19 +167,16 @@
     return listener;
 }
 
-Status changeOwnerAndMode(const char* path, gid_t group, const char* debugName, bool netdOnly) {
+Status changeOwnerAndMode(const char* path, gid_t group, const char* debugName, mode_t groupMode) {
     int ret = chown(path, AID_ROOT, group);
     if (ret != 0) return statusFromErrno(errno, StringPrintf("change %s group failed", debugName));
 
-    if (netdOnly) {
-        ret = chmod(path, S_IRWXU);
-    } else {
-        // Allow both netd and system server to obtain map fd from the path.
-        // chmod doesn't grant permission to all processes in that group to
-        // read/write the bpf map. They still need correct sepolicy to
-        // read/write the map.
-        ret = chmod(path, S_IRWXU | S_IRGRP | S_IWGRP);
-    }
+    // Ensure groupMode only contains group bits.
+    groupMode &= S_IRGRP | S_IWGRP;
+
+    // chmod doesn't by itself grant permission to all processes in that group to
+    // read/write the bpf map. They still need correct sepolicy.
+    ret = chmod(path, S_IRUSR | S_IWUSR | groupMode);
     if (ret != 0) return statusFromErrno(errno, StringPrintf("change %s mode failed", debugName));
     return netdutils::status::ok;
 }
@@ -196,43 +194,56 @@
 Status TrafficController::initMaps() {
     std::lock_guard guard(mMutex);
     RETURN_IF_NOT_OK(mCookieTagMap.init(COOKIE_TAG_MAP_PATH));
-    RETURN_IF_NOT_OK(changeOwnerAndMode(COOKIE_TAG_MAP_PATH, AID_NET_BW_ACCT, "CookieTagMap",
-                                        false));
+    RETURN_IF_NOT_OK(
+            changeOwnerAndMode(COOKIE_TAG_MAP_PATH, AID_NET_BW_ACCT, "CookieTagMap", S_IRGRP));
 
     RETURN_IF_NOT_OK(mUidCounterSetMap.init(UID_COUNTERSET_MAP_PATH));
     RETURN_IF_NOT_OK(changeOwnerAndMode(UID_COUNTERSET_MAP_PATH, AID_NET_BW_ACCT,
-                                        "UidCounterSetMap", false));
+                                        "UidCounterSetMap", S_IRGRP));
 
     RETURN_IF_NOT_OK(mAppUidStatsMap.init(APP_UID_STATS_MAP_PATH));
-    RETURN_IF_NOT_OK(
-        changeOwnerAndMode(APP_UID_STATS_MAP_PATH, AID_NET_BW_STATS, "AppUidStatsMap", false));
+    RETURN_IF_NOT_OK(changeOwnerAndMode(APP_UID_STATS_MAP_PATH, AID_NET_BW_STATS, "AppUidStatsMap",
+                                        S_IRGRP));
 
     RETURN_IF_NOT_OK(mStatsMapA.init(STATS_MAP_A_PATH));
-    RETURN_IF_NOT_OK(changeOwnerAndMode(STATS_MAP_A_PATH, AID_NET_BW_STATS, "StatsMapA", false));
+    RETURN_IF_NOT_OK(
+            changeOwnerAndMode(STATS_MAP_A_PATH, AID_NET_BW_STATS, "StatsMapA", S_IRGRP | S_IWGRP));
 
     RETURN_IF_NOT_OK(mStatsMapB.init(STATS_MAP_B_PATH));
-    RETURN_IF_NOT_OK(changeOwnerAndMode(STATS_MAP_B_PATH, AID_NET_BW_STATS, "StatsMapB", false));
+    RETURN_IF_NOT_OK(
+            changeOwnerAndMode(STATS_MAP_B_PATH, AID_NET_BW_STATS, "StatsMapB", S_IRGRP | S_IWGRP));
 
     RETURN_IF_NOT_OK(mIfaceIndexNameMap.init(IFACE_INDEX_NAME_MAP_PATH));
     RETURN_IF_NOT_OK(changeOwnerAndMode(IFACE_INDEX_NAME_MAP_PATH, AID_NET_BW_STATS,
-                                        "IfaceIndexNameMap", false));
+                                        "IfaceIndexNameMap", S_IRGRP));
 
     RETURN_IF_NOT_OK(mIfaceStatsMap.init(IFACE_STATS_MAP_PATH));
-    RETURN_IF_NOT_OK(changeOwnerAndMode(IFACE_STATS_MAP_PATH, AID_NET_BW_STATS, "IfaceStatsMap",
-                                        false));
+    RETURN_IF_NOT_OK(
+            changeOwnerAndMode(IFACE_STATS_MAP_PATH, AID_NET_BW_STATS, "IfaceStatsMap", S_IRGRP));
 
     RETURN_IF_NOT_OK(mConfigurationMap.init(CONFIGURATION_MAP_PATH));
     RETURN_IF_NOT_OK(changeOwnerAndMode(CONFIGURATION_MAP_PATH, AID_NET_BW_STATS,
-                                        "ConfigurationMap", false));
+                                        "ConfigurationMap", S_IRGRP));
     RETURN_IF_NOT_OK(
             mConfigurationMap.writeValue(UID_RULES_CONFIGURATION_KEY, DEFAULT_CONFIG, BPF_ANY));
     RETURN_IF_NOT_OK(mConfigurationMap.writeValue(CURRENT_STATS_MAP_CONFIGURATION_KEY, SELECT_MAP_A,
                                                   BPF_ANY));
 
     RETURN_IF_NOT_OK(mUidOwnerMap.init(UID_OWNER_MAP_PATH));
-    RETURN_IF_NOT_OK(changeOwnerAndMode(UID_OWNER_MAP_PATH, AID_ROOT, "UidOwnerMap", true));
+    RETURN_IF_NOT_OK(changeOwnerAndMode(UID_OWNER_MAP_PATH, AID_ROOT, "UidOwnerMap", S_NONE));
     RETURN_IF_NOT_OK(mUidOwnerMap.clear());
     RETURN_IF_NOT_OK(mUidPermissionMap.init(UID_PERMISSION_MAP_PATH));
+
+    // The programs must be readable to process that modify iptables rules
+    RETURN_IF_NOT_OK(changeOwnerAndMode(XT_BPF_EGRESS_PROG_PATH, AID_NET_ADMIN,
+                                        "XtFilterEgressProgram", S_IRGRP));
+    RETURN_IF_NOT_OK(changeOwnerAndMode(XT_BPF_INGRESS_PROG_PATH, AID_NET_ADMIN,
+                                        "XtFilterIngressProgram", S_IRGRP));
+    RETURN_IF_NOT_OK(changeOwnerAndMode(XT_BPF_WHITELIST_PROG_PATH, AID_NET_ADMIN,
+                                        "XtWhitelistProgram", S_IRGRP));
+    RETURN_IF_NOT_OK(changeOwnerAndMode(XT_BPF_BLACKLIST_PROG_PATH, AID_NET_ADMIN,
+                                        "XtBlacklistProgram", S_IRGRP));
+
     return netdutils::status::ok;
 }