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
This commit is contained in:
markchien 2021-09-29 22:57:31 +08:00
commit 28160b3141
2 changed files with 42 additions and 0 deletions

View file

@ -5467,6 +5467,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
@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 @@ public class ConnectivityService extends IConnectivityManager.Stub
@Override
@Deprecated
public String[] getTetherableIfaces() {
enforceAccessPermission();
final TetheringManager tm = (TetheringManager) mContext.getSystemService(
Context.TETHERING_SERVICE);
return tm.getTetherableIfaces();
@ -5483,6 +5485,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
@Override
@Deprecated
public String[] getTetheredIfaces() {
enforceAccessPermission();
final TetheringManager tm = (TetheringManager) mContext.getSystemService(
Context.TETHERING_SERVICE);
return tm.getTetheredIfaces();
@ -5492,6 +5495,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
@Override
@Deprecated
public String[] getTetheringErroredIfaces() {
enforceAccessPermission();
final TetheringManager tm = (TetheringManager) mContext.getSystemService(
Context.TETHERING_SERVICE);
@ -5501,6 +5505,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
@Override
@Deprecated
public String[] getTetherableUsbRegexs() {
enforceAccessPermission();
final TetheringManager tm = (TetheringManager) mContext.getSystemService(
Context.TETHERING_SERVICE);
@ -5510,6 +5515,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
@Override
@Deprecated
public String[] getTetherableWifiRegexs() {
enforceAccessPermission();
final TetheringManager tm = (TetheringManager) mContext.getSystemService(
Context.TETHERING_SERVICE);
return tm.getTetherableWifiRegexs();

View file

@ -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.RouteInfo;
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 @@ public class ConnectivityServiceTest {
@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 @@ public class ConnectivityServiceTest {
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 @@ public class ConnectivityServiceTest {
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();
});
}
}