Fix permission bypass problem for Tethering deprecated APIs
Since the tethering functions in ConnectivityService is delegated
to TetheringManager instance and get caches informataion in
TetheringManager without checking ACCESS_NETWORK_STATE permission.
If application use reflection call getTetherXXX functions in
ConnectivityService, it can get tethering status with no additional
execution privileges needed.
Bug: 162952629
Test: manual
Ignore-AOSP-First: security fix
Change-Id: I5b897f216db19fead6ba6ac07915aa0f6ff5bf42
diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java
index a0e75ec..81bb4f5 100644
--- a/service/src/com/android/server/ConnectivityService.java
+++ b/service/src/com/android/server/ConnectivityService.java
@@ -5467,6 +5467,7 @@
@Override
@Deprecated
public int getLastTetherError(String iface) {
+ enforceAccessPermission();
final TetheringManager tm = (TetheringManager) mContext.getSystemService(
Context.TETHERING_SERVICE);
return tm.getLastTetherError(iface);
@@ -5475,6 +5476,7 @@
@Override
@Deprecated
public String[] getTetherableIfaces() {
+ enforceAccessPermission();
final TetheringManager tm = (TetheringManager) mContext.getSystemService(
Context.TETHERING_SERVICE);
return tm.getTetherableIfaces();
@@ -5483,6 +5485,7 @@
@Override
@Deprecated
public String[] getTetheredIfaces() {
+ enforceAccessPermission();
final TetheringManager tm = (TetheringManager) mContext.getSystemService(
Context.TETHERING_SERVICE);
return tm.getTetheredIfaces();
@@ -5492,6 +5495,7 @@
@Override
@Deprecated
public String[] getTetheringErroredIfaces() {
+ enforceAccessPermission();
final TetheringManager tm = (TetheringManager) mContext.getSystemService(
Context.TETHERING_SERVICE);
@@ -5501,6 +5505,7 @@
@Override
@Deprecated
public String[] getTetherableUsbRegexs() {
+ enforceAccessPermission();
final TetheringManager tm = (TetheringManager) mContext.getSystemService(
Context.TETHERING_SERVICE);
@@ -5510,6 +5515,7 @@
@Override
@Deprecated
public String[] getTetherableWifiRegexs() {
+ enforceAccessPermission();
final TetheringManager tm = (TetheringManager) mContext.getSystemService(
Context.TETHERING_SERVICE);
return tm.getTetherableWifiRegexs();
diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java
index 4c76803..a4ee78f 100644
--- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java
+++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java
@@ -16,6 +16,7 @@
package com.android.server;
+import static android.Manifest.permission.ACCESS_NETWORK_STATE;
import static android.Manifest.permission.CHANGE_NETWORK_STATE;
import static android.Manifest.permission.CONNECTIVITY_USE_RESTRICTED_NETWORKS;
import static android.Manifest.permission.CONTROL_OEM_PAID_NETWORK_PREFERENCE;
@@ -269,6 +270,7 @@
import android.net.RouteInfoParcel;
import android.net.SocketKeepalive;
import android.net.TelephonyNetworkSpecifier;
+import android.net.TetheringManager;
import android.net.TransportInfo;
import android.net.UidRange;
import android.net.UidRangeParcel;
@@ -543,6 +545,7 @@
@Mock PacProxyManager mPacProxyManager;
@Mock BpfNetMaps mBpfNetMaps;
@Mock CarrierPrivilegeAuthenticator mCarrierPrivilegeAuthenticator;
+ @Mock TetheringManager mTetheringManager;
// BatteryStatsManager is final and cannot be mocked with regular mockito, so just mock the
// underlying binder calls.
@@ -663,6 +666,7 @@
if (Context.NETWORK_STATS_SERVICE.equals(name)) return mStatsManager;
if (Context.BATTERY_STATS_SERVICE.equals(name)) return mBatteryStatsManager;
if (Context.PAC_PROXY_SERVICE.equals(name)) return mPacProxyManager;
+ if (Context.TETHERING_SERVICE.equals(name)) return mTetheringManager;
return super.getSystemService(name);
}
@@ -15699,4 +15703,36 @@
mCm.reportNetworkConnectivity(mWiFiNetworkAgent.getNetwork(), false);
mDefaultNetworkCallback.expectAvailableCallbacksValidated(mCellNetworkAgent);
}
+
+ @Test
+ public void testLegacyTetheringApiGuardWithProperPermission() throws Exception {
+ final String testIface = "test0";
+ mServiceContext.setPermission(ACCESS_NETWORK_STATE, PERMISSION_DENIED);
+ assertThrows(SecurityException.class, () -> mService.getLastTetherError(testIface));
+ assertThrows(SecurityException.class, () -> mService.getTetherableIfaces());
+ assertThrows(SecurityException.class, () -> mService.getTetheredIfaces());
+ assertThrows(SecurityException.class, () -> mService.getTetheringErroredIfaces());
+ assertThrows(SecurityException.class, () -> mService.getTetherableUsbRegexs());
+ assertThrows(SecurityException.class, () -> mService.getTetherableWifiRegexs());
+
+ withPermission(ACCESS_NETWORK_STATE, () -> {
+ mService.getLastTetherError(testIface);
+ verify(mTetheringManager).getLastTetherError(testIface);
+
+ mService.getTetherableIfaces();
+ verify(mTetheringManager).getTetherableIfaces();
+
+ mService.getTetheredIfaces();
+ verify(mTetheringManager).getTetheredIfaces();
+
+ mService.getTetheringErroredIfaces();
+ verify(mTetheringManager).getTetheringErroredIfaces();
+
+ mService.getTetherableUsbRegexs();
+ verify(mTetheringManager).getTetherableUsbRegexs();
+
+ mService.getTetherableWifiRegexs();
+ verify(mTetheringManager).getTetherableWifiRegexs();
+ });
+ }
}