From 6e4c4f47eaf509d4c0a8a1af39b4b5f3c84a9098 Mon Sep 17 00:00:00 2001 From: xshu Date: Fri, 20 Apr 2018 15:37:44 -0700 Subject: Make NETWORK_SETTINGS trump CHANGE_WIFI_STATE NETWORK_SETTINGS is a more powerful permission. If a caller has NETWORK_SETTINGS than we no longer should check if it has CHANGE_WIFI_STATE. Bug: 78228349 Test: compile, unit test manual test: flash to device go to settings -> apps & notifications -> special app access -> Wi-Fi control top right corner -> show system Change permission for every item to "Not allowed" Restart phone Observe a successful boot Change-Id: Ie5543c745aa1f6851eef8d04408c5691dcc54fb0 --- .../com/android/server/wifi/WifiServiceImpl.java | 3 + .../android/server/wifi/WifiServiceImplTest.java | 78 ++++++++++++++++++++-- 2 files changed, 76 insertions(+), 5 deletions(-) diff --git a/service/java/com/android/server/wifi/WifiServiceImpl.java b/service/java/com/android/server/wifi/WifiServiceImpl.java index 5d4df7df5..1a5e0e27d 100644 --- a/service/java/com/android/server/wifi/WifiServiceImpl.java +++ b/service/java/com/android/server/wifi/WifiServiceImpl.java @@ -725,6 +725,9 @@ public class WifiServiceImpl extends IWifiManager.Stub { */ @CheckResult private int enforceChangePermission(String callingPackage) { + if (checkNetworkSettingsPermission(Binder.getCallingPid(), Binder.getCallingUid())) { + return MODE_ALLOWED; + } mContext.enforceCallingOrSelfPermission(android.Manifest.permission.CHANGE_WIFI_STATE, "WifiService"); diff --git a/tests/wifitests/src/com/android/server/wifi/WifiServiceImplTest.java b/tests/wifitests/src/com/android/server/wifi/WifiServiceImplTest.java index 996363916..136d029d3 100644 --- a/tests/wifitests/src/com/android/server/wifi/WifiServiceImplTest.java +++ b/tests/wifitests/src/com/android/server/wifi/WifiServiceImplTest.java @@ -309,6 +309,8 @@ public class WifiServiceImplTest { // Create an OSU provider that can be provisioned via an open OSU AP mOsuProvider = PasspointProvisioningTestUtil.generateOsuProvider(true); when(mContext.getOpPackageName()).thenReturn(TEST_PACKAGE_NAME); + when(mContext.checkPermission(eq(android.Manifest.permission.NETWORK_SETTINGS), + anyInt(), anyInt())).thenReturn(PackageManager.PERMISSION_DENIED); ArgumentCaptor softApCallbackCaptor = ArgumentCaptor.forClass(SoftApCallback.class); @@ -400,26 +402,36 @@ public class WifiServiceImplTest { * Verify a SecurityException is thrown if a caller does not have the correct permission to * toggle wifi. */ - @Test(expected = SecurityException.class) + @Test public void testSetWifiEnableWithoutPermission() throws Exception { doThrow(new SecurityException()).when(mContext) .enforceCallingOrSelfPermission(eq(android.Manifest.permission.CHANGE_WIFI_STATE), eq("WifiService")); when(mSettingsStore.isAirplaneModeOn()).thenReturn(false); - mWifiServiceImpl.setWifiEnabled(TEST_PACKAGE_NAME, true); + try { + mWifiServiceImpl.setWifiEnabled(TEST_PACKAGE_NAME, true); + fail(); + } catch (SecurityException e) { + + } + } /** * Verify a SecurityException is thrown if OPSTR_CHANGE_WIFI_STATE is disabled for the app. */ - @Test(expected = SecurityException.class) + @Test public void testSetWifiEnableAppOpsRejected() throws Exception { when(mSettingsStore.handleWifiToggled(eq(true))).thenReturn(true); doThrow(new SecurityException()).when(mAppOpsManager) .noteOp(AppOpsManager.OPSTR_CHANGE_WIFI_STATE, Process.myUid(), TEST_PACKAGE_NAME); - when(mSettingsStore.isAirplaneModeOn()).thenReturn(false); - mWifiServiceImpl.setWifiEnabled(TEST_PACKAGE_NAME, true); + try { + mWifiServiceImpl.setWifiEnabled(TEST_PACKAGE_NAME, true); + fail(); + } catch (SecurityException e) { + + } verify(mWifiController, never()).sendMessage(eq(CMD_WIFI_TOGGLED)); } @@ -2534,6 +2546,55 @@ public class WifiServiceImplTest { verify(mScanRequestProxy).startScan(Process.myUid(), SCAN_PACKAGE_NAME); } + /** + * Verify that if the caller has NETWORK_SETTINGS permission, then it doesn't need + * CHANGE_WIFI_STATE permission. + * @throws Exception + */ + @Test + public void testDisconnectWithNetworkSettingsPerm() throws Exception { + when(mContext.checkPermission(eq(android.Manifest.permission.NETWORK_SETTINGS), + anyInt(), anyInt())).thenReturn(PackageManager.PERMISSION_GRANTED); + doThrow(new SecurityException()).when(mContext).enforceCallingOrSelfPermission( + android.Manifest.permission.CHANGE_WIFI_STATE, "WifiService"); + doThrow(new SecurityException()).when(mAppOpsManager) + .noteOp(AppOpsManager.OPSTR_CHANGE_WIFI_STATE, Process.myUid(), TEST_PACKAGE_NAME); + mWifiServiceImpl.disconnect(TEST_PACKAGE_NAME); + verify(mWifiStateMachine).disconnectCommand(); + } + + /** + * Verify that if the caller doesn't have NETWORK_SETTINGS permission, it could still + * get access with the CHANGE_WIFI_STATE permission. + * @throws Exception + */ + @Test + public void testDisconnectWithChangeWifiStatePerm() throws Exception { + mWifiServiceImpl.disconnect(TEST_PACKAGE_NAME); + verifyCheckChangePermission(TEST_PACKAGE_NAME); + verify(mWifiStateMachine).disconnectCommand(); + } + + /** + * Verify that the operation fails if the caller has neither NETWORK_SETTINGS or + * CHANGE_WIFI_STATE permissions. + * @throws Exception + */ + @Test + public void testDisconnectRejected() throws Exception { + doThrow(new SecurityException()).when(mAppOpsManager) + .noteOp(AppOpsManager.OPSTR_CHANGE_WIFI_STATE, Process.myUid(), TEST_PACKAGE_NAME); + try { + mWifiServiceImpl.disconnect(TEST_PACKAGE_NAME); + fail(); + } catch (SecurityException e) { + + } + verifyCheckChangePermission(TEST_PACKAGE_NAME); + verify(mWifiStateMachine, never()).disconnectCommand(); + } + + private class IdleModeIntentMatcher implements ArgumentMatcher { @Override public boolean matches(IntentFilter filter) { @@ -2541,7 +2602,14 @@ public class WifiServiceImplTest { } } + /** + * Verifies that enforceChangePermission(String package) is called and the caller doesn't + * have NETWORK_SETTINGS permission + */ private void verifyCheckChangePermission(String callingPackageName) { + verify(mContext, atLeastOnce()) + .checkPermission(eq(android.Manifest.permission.NETWORK_SETTINGS), + anyInt(), anyInt()); verify(mContext, atLeastOnce()).enforceCallingOrSelfPermission( android.Manifest.permission.CHANGE_WIFI_STATE, "WifiService"); verify(mAppOpsManager, atLeastOnce()).noteOp( -- cgit v1.2.3