From 28160b3141286f7b579696776fbce7eeeff00b6f Mon Sep 17 00:00:00 2001 From: markchien Date: Wed, 29 Sep 2021 22:57:31 +0800 Subject: [PATCH] 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 --- .../android/server/ConnectivityService.java | 6 ++++ .../server/ConnectivityServiceTest.java | 36 +++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index a0e75ec048..81bb4f59fe 100644 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -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(); diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java index 4c768030f8..a4ee78f3ce 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.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(); + }); + } }