Enforce permission check in getUidStats function

The NetworkStatsService.getUidStats() currently doesn't have any
permission check to make sure unpriviledged apps cannot read the stats
of a different uid. It will protentially have security problem since
apps with ACCESS_NETWORK_STATS permission can directly calling into
NetworkStatsService and bypass the check in TrafficStats. Move the uid
check from TrafficStats to NetworkStatsService to fix the problem.

Bug: 129151407
Test: atest AppSecurityTests#testAppFailAccessPrivateData_full
Test: atest AppSecurityTests#testAppFailAccessPrivateData_instant
Test: atest android.app.usage.cts.NetworkUsageStatsTest
Test: atest NetworkStatsBinderTest

Change-Id: Iae85676cfe5f114da69ec278afc2c904bc907234
This commit is contained in:
Chenbo Feng
2019-06-17 16:22:28 -07:00
committed by junyulai
parent 5d0d638f54
commit d80a6ed9e2
2 changed files with 21 additions and 44 deletions

View File

@@ -775,17 +775,10 @@ public class TrafficStats {
* @see android.content.pm.ApplicationInfo#uid * @see android.content.pm.ApplicationInfo#uid
*/ */
public static long getUidTxBytes(int uid) { public static long getUidTxBytes(int uid) {
// This isn't actually enforcing any security; it just returns the try {
// unsupported value. The real filtering is done at the kernel level. return getStatsService().getUidStats(uid, TYPE_TX_BYTES);
final int callingUid = android.os.Process.myUid(); } catch (RemoteException e) {
if (callingUid == android.os.Process.SYSTEM_UID || callingUid == uid) { throw e.rethrowFromSystemServer();
try {
return getStatsService().getUidStats(uid, TYPE_TX_BYTES);
} catch (RemoteException e) {
throw e.rethrowFromSystemServer();
}
} else {
return UNSUPPORTED;
} }
} }
@@ -808,17 +801,10 @@ public class TrafficStats {
* @see android.content.pm.ApplicationInfo#uid * @see android.content.pm.ApplicationInfo#uid
*/ */
public static long getUidRxBytes(int uid) { public static long getUidRxBytes(int uid) {
// This isn't actually enforcing any security; it just returns the try {
// unsupported value. The real filtering is done at the kernel level. return getStatsService().getUidStats(uid, TYPE_RX_BYTES);
final int callingUid = android.os.Process.myUid(); } catch (RemoteException e) {
if (callingUid == android.os.Process.SYSTEM_UID || callingUid == uid) { throw e.rethrowFromSystemServer();
try {
return getStatsService().getUidStats(uid, TYPE_RX_BYTES);
} catch (RemoteException e) {
throw e.rethrowFromSystemServer();
}
} else {
return UNSUPPORTED;
} }
} }
@@ -841,17 +827,10 @@ public class TrafficStats {
* @see android.content.pm.ApplicationInfo#uid * @see android.content.pm.ApplicationInfo#uid
*/ */
public static long getUidTxPackets(int uid) { public static long getUidTxPackets(int uid) {
// This isn't actually enforcing any security; it just returns the try {
// unsupported value. The real filtering is done at the kernel level. return getStatsService().getUidStats(uid, TYPE_TX_PACKETS);
final int callingUid = android.os.Process.myUid(); } catch (RemoteException e) {
if (callingUid == android.os.Process.SYSTEM_UID || callingUid == uid) { throw e.rethrowFromSystemServer();
try {
return getStatsService().getUidStats(uid, TYPE_TX_PACKETS);
} catch (RemoteException e) {
throw e.rethrowFromSystemServer();
}
} else {
return UNSUPPORTED;
} }
} }
@@ -874,17 +853,10 @@ public class TrafficStats {
* @see android.content.pm.ApplicationInfo#uid * @see android.content.pm.ApplicationInfo#uid
*/ */
public static long getUidRxPackets(int uid) { public static long getUidRxPackets(int uid) {
// This isn't actually enforcing any security; it just returns the try {
// unsupported value. The real filtering is done at the kernel level. return getStatsService().getUidStats(uid, TYPE_RX_PACKETS);
final int callingUid = android.os.Process.myUid(); } catch (RemoteException e) {
if (callingUid == android.os.Process.SYSTEM_UID || callingUid == uid) { throw e.rethrowFromSystemServer();
try {
return getStatsService().getUidStats(uid, TYPE_RX_PACKETS);
} catch (RemoteException e) {
throw e.rethrowFromSystemServer();
}
} else {
return UNSUPPORTED;
} }
} }

View File

@@ -49,6 +49,7 @@ import static android.net.NetworkTemplate.buildTemplateWifiWildcard;
import static android.net.NetworkTemplate.getCollapsedRatType; import static android.net.NetworkTemplate.getCollapsedRatType;
import static android.net.TrafficStats.KB_IN_BYTES; import static android.net.TrafficStats.KB_IN_BYTES;
import static android.net.TrafficStats.MB_IN_BYTES; import static android.net.TrafficStats.MB_IN_BYTES;
import static android.net.TrafficStats.UNSUPPORTED;
import static android.os.Trace.TRACE_TAG_NETWORK; import static android.os.Trace.TRACE_TAG_NETWORK;
import static android.provider.Settings.Global.NETSTATS_AUGMENT_ENABLED; import static android.provider.Settings.Global.NETSTATS_AUGMENT_ENABLED;
import static android.provider.Settings.Global.NETSTATS_COMBINE_SUBTYPE_ENABLED; import static android.provider.Settings.Global.NETSTATS_COMBINE_SUBTYPE_ENABLED;
@@ -1021,6 +1022,10 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
@Override @Override
public long getUidStats(int uid, int type) { public long getUidStats(int uid, int type) {
final int callingUid = Binder.getCallingUid();
if (callingUid != android.os.Process.SYSTEM_UID && callingUid != uid) {
return UNSUPPORTED;
}
return nativeGetUidStat(uid, type, checkBpfStatsEnable()); return nativeGetUidStat(uid, type, checkBpfStatsEnable());
} }