diff options
8 files changed, 110 insertions, 82 deletions
diff --git a/service/java/com/android/server/wifi/WifiConfigManager.java b/service/java/com/android/server/wifi/WifiConfigManager.java index cd10bda67..7c8cddae8 100644 --- a/service/java/com/android/server/wifi/WifiConfigManager.java +++ b/service/java/com/android/server/wifi/WifiConfigManager.java @@ -35,7 +35,6 @@ import android.net.wifi.WifiInfo; import android.net.wifi.WifiManager; import android.net.wifi.WifiScanner; import android.os.Process; -import android.os.RemoteException; import android.os.UserHandle; import android.os.UserManager; import android.provider.Settings; @@ -52,6 +51,7 @@ import com.android.server.wifi.WifiConfigStoreLegacy.WifiConfigStoreDataLegacy; import com.android.server.wifi.hotspot2.PasspointManager; import com.android.server.wifi.util.ScanResultUtil; import com.android.server.wifi.util.TelephonyUtil; +import com.android.server.wifi.util.WifiPermissionsUtil; import com.android.server.wifi.util.WifiPermissionsWrapper; import org.xmlpull.v1.XmlPullParserException; @@ -211,7 +211,6 @@ public class WifiConfigManager { * List of external dependencies for WifiConfigManager. */ private final Context mContext; - private final FrameworkFacade mFacade; private final Clock mClock; private final UserManager mUserManager; private final BackupManagerProxy mBackupManagerProxy; @@ -219,6 +218,7 @@ public class WifiConfigManager { private final WifiKeyStore mWifiKeyStore; private final WifiConfigStore mWifiConfigStore; private final WifiConfigStoreLegacy mWifiConfigStoreLegacy; + private final WifiPermissionsUtil mWifiPermissionsUtil; private final WifiPermissionsWrapper mWifiPermissionsWrapper; /** * Local log used for debugging any WifiConfigManager issues. @@ -302,14 +302,14 @@ public class WifiConfigManager { * Create new instance of WifiConfigManager. */ WifiConfigManager( - Context context, FrameworkFacade facade, Clock clock, UserManager userManager, + Context context, Clock clock, UserManager userManager, TelephonyManager telephonyManager, WifiKeyStore wifiKeyStore, WifiConfigStore wifiConfigStore, WifiConfigStoreLegacy wifiConfigStoreLegacy, + WifiPermissionsUtil wifiPermissionsUtil, WifiPermissionsWrapper wifiPermissionsWrapper, NetworkListStoreData networkListStoreData, DeletedEphemeralSsidsStoreData deletedEphemeralSsidsStoreData) { mContext = context; - mFacade = facade; mClock = clock; mUserManager = userManager; mBackupManagerProxy = new BackupManagerProxy(); @@ -317,6 +317,7 @@ public class WifiConfigManager { mWifiKeyStore = wifiKeyStore; mWifiConfigStore = wifiConfigStore; mWifiConfigStoreLegacy = wifiConfigStoreLegacy; + mWifiPermissionsUtil = wifiPermissionsUtil; mWifiPermissionsWrapper = wifiPermissionsWrapper; mConfiguredNetworks = new ConfigurationMap(userManager); @@ -606,24 +607,6 @@ public class WifiConfigManager { } /** - * Checks if the app has the permission to override Wi-Fi network configuration or not. - * - * @param uid uid of the app. - * @return true if the app does have the permission, false otherwise. - */ - public boolean checkConfigOverridePermission(int uid) { - try { - int permission = - mFacade.checkUidPermission( - android.Manifest.permission.OVERRIDE_WIFI_CONFIG, uid); - return (permission == PackageManager.PERMISSION_GRANTED); - } catch (RemoteException e) { - Log.e(TAG, "Error checking for permission " + e); - return false; - } - } - - /** * Checks if |uid| has permission to modify the provided configuration. * * @param config WifiConfiguration object corresponding to the network to be modified. @@ -647,7 +630,7 @@ public class WifiConfigManager { // Check if the |uid| holds the |OVERRIDE_CONFIG_WIFI| permission if the caller asks us to // bypass the lockdown checks. if (ignoreLockdown) { - return checkConfigOverridePermission(uid); + return mWifiPermissionsUtil.checkConfigOverridePermission(uid); } // Check if device has DPM capability. If it has and |dpmi| is still null, then we @@ -662,13 +645,13 @@ public class WifiConfigManager { final boolean isConfigEligibleForLockdown = dpmi != null && dpmi.isActiveAdminWithPolicy( config.creatorUid, DeviceAdminInfo.USES_POLICY_DEVICE_OWNER); if (!isConfigEligibleForLockdown) { - return isCreator || checkConfigOverridePermission(uid); + return isCreator || mWifiPermissionsUtil.checkConfigOverridePermission(uid); } final ContentResolver resolver = mContext.getContentResolver(); final boolean isLockdownFeatureEnabled = Settings.Global.getInt(resolver, Settings.Global.WIFI_DEVICE_OWNER_CONFIGS_LOCKDOWN, 0) != 0; - return !isLockdownFeatureEnabled && checkConfigOverridePermission(uid); + return !isLockdownFeatureEnabled && mWifiPermissionsUtil.checkConfigOverridePermission(uid); } /** @@ -2777,7 +2760,8 @@ public class WifiConfigManager { DeviceAdminInfo.USES_POLICY_PROFILE_OWNER); final boolean isUidDeviceOwner = dpmi != null && dpmi.isActiveAdminWithPolicy(uid, DeviceAdminInfo.USES_POLICY_DEVICE_OWNER); - final boolean hasConfigOverridePermission = checkConfigOverridePermission(uid); + final boolean hasConfigOverridePermission = + mWifiPermissionsUtil.checkConfigOverridePermission(uid); // If |uid| corresponds to the device owner, allow all modifications. if (isUidDeviceOwner || isUidProfileOwner || hasConfigOverridePermission) { return true; diff --git a/service/java/com/android/server/wifi/WifiInjector.java b/service/java/com/android/server/wifi/WifiInjector.java index 59c1a5ac3..f41f5f90b 100644 --- a/service/java/com/android/server/wifi/WifiInjector.java +++ b/service/java/com/android/server/wifi/WifiInjector.java @@ -190,10 +190,11 @@ public class WifiInjector { mWifiNetworkHistory, mWifiNative, mIpConfigStore, new LegacyPasspointConfigParser()); // Config Manager - mWifiConfigManager = new WifiConfigManager(mContext, mFrameworkFacade, mClock, + mWifiConfigManager = new WifiConfigManager(mContext, mClock, UserManager.get(mContext), TelephonyManager.from(mContext), - mWifiKeyStore, mWifiConfigStore, mWifiConfigStoreLegacy, mWifiPermissionsWrapper, - new NetworkListStoreData(), new DeletedEphemeralSsidsStoreData()); + mWifiKeyStore, mWifiConfigStore, mWifiConfigStoreLegacy, mWifiPermissionsUtil, + mWifiPermissionsWrapper, new NetworkListStoreData(), + new DeletedEphemeralSsidsStoreData()); mWifiNetworkSelector = new WifiNetworkSelector(mContext, mWifiConfigManager, mClock); LocalLog localLog = mWifiNetworkSelector.getLocalLog(); mSavedNetworkEvaluator = new SavedNetworkEvaluator(mContext, diff --git a/service/java/com/android/server/wifi/WifiStateMachine.java b/service/java/com/android/server/wifi/WifiStateMachine.java index 10f38aa88..ccbc8e811 100644 --- a/service/java/com/android/server/wifi/WifiStateMachine.java +++ b/service/java/com/android/server/wifi/WifiStateMachine.java @@ -118,6 +118,7 @@ import com.android.server.wifi.util.NativeUtil; import com.android.server.wifi.util.TelephonyUtil; import com.android.server.wifi.util.TelephonyUtil.SimAuthRequestData; import com.android.server.wifi.util.TelephonyUtil.SimAuthResponseData; +import com.android.server.wifi.util.WifiPermissionsUtil; import java.io.BufferedReader; import java.io.FileDescriptor; @@ -198,6 +199,7 @@ public class WifiStateMachine extends StateMachine implements WifiNative.WifiRss private WifiInjector mWifiInjector; private WifiMonitor mWifiMonitor; private WifiNative mWifiNative; + private WifiPermissionsUtil mWifiPermissionsUtil; private WifiConfigManager mWifiConfigManager; private WifiConnectivityManager mWifiConnectivityManager; private WifiNetworkSelector mWifiNetworkSelector; @@ -885,6 +887,7 @@ public class WifiStateMachine extends StateMachine implements WifiNative.WifiRss mP2pSupported = mContext.getPackageManager().hasSystemFeature( PackageManager.FEATURE_WIFI_DIRECT); + mWifiPermissionsUtil = mWifiInjector.getWifiPermissionsUtil(); mWifiConfigManager = mWifiInjector.getWifiConfigManager(); mWifiApConfigStore = mWifiInjector.getWifiApConfigStore(); mWifiNative.setSystemSupportsFastBssTransition( @@ -5852,7 +5855,7 @@ public class WifiStateMachine extends StateMachine implements WifiNative.WifiRss WifiConfiguration config = getCurrentWifiConfiguration(); if (mWifiConfigManager.getLastSelectedNetwork() == config.networkId) { boolean prompt = - mWifiConfigManager.checkConfigOverridePermission(config.lastConnectUid); + mWifiPermissionsUtil.checkConfigOverridePermission(config.lastConnectUid); if (mVerboseLoggingEnabled) { log("Network selected by UID " + config.lastConnectUid + " prompt=" + prompt); } diff --git a/service/java/com/android/server/wifi/util/WifiPermissionsUtil.java b/service/java/com/android/server/wifi/util/WifiPermissionsUtil.java index 3d473d44f..2010dcfa7 100644 --- a/service/java/com/android/server/wifi/util/WifiPermissionsUtil.java +++ b/service/java/com/android/server/wifi/util/WifiPermissionsUtil.java @@ -22,6 +22,7 @@ import android.content.Context; import android.content.pm.PackageManager; import android.content.pm.UserInfo; import android.net.NetworkScoreManager; +import android.os.RemoteException; import android.os.UserManager; import android.provider.Settings; @@ -58,6 +59,22 @@ public class WifiPermissionsUtil { } /** + * Checks if the app has the permission to override Wi-Fi network configuration or not. + * + * @param uid uid of the app. + * @return true if the app does have the permission, false otherwise. + */ + public boolean checkConfigOverridePermission(int uid) { + try { + int permission = mWifiPermissionsWrapper.getOverrideWifiConfigPermission(uid); + return (permission == PackageManager.PERMISSION_GRANTED); + } catch (RemoteException e) { + mLog.err("Error checking for permission: %").r(e.getMessage()).flush(); + return false; + } + } + + /** * API to determine if the caller has permissions to get * scan results. * @param pkgName Packagename of the application requesting access diff --git a/service/java/com/android/server/wifi/util/WifiPermissionsWrapper.java b/service/java/com/android/server/wifi/util/WifiPermissionsWrapper.java index 72c439d47..6ca2f0291 100644 --- a/service/java/com/android/server/wifi/util/WifiPermissionsWrapper.java +++ b/service/java/com/android/server/wifi/util/WifiPermissionsWrapper.java @@ -17,8 +17,10 @@ package com.android.server.wifi.util; import android.app.ActivityManager; +import android.app.AppGlobals; import android.app.admin.DevicePolicyManagerInternal; import android.content.Context; +import android.os.RemoteException; import android.os.UserHandle; import com.android.server.LocalServices; @@ -81,4 +83,16 @@ public class WifiPermissionsWrapper { public DevicePolicyManagerInternal getDevicePolicyManagerInternal() { return LocalServices.getService(DevicePolicyManagerInternal.class); } + + /** + * Determines if the caller has the override wifi config permission. + * + * @param uid to check the permission for + * @return int representation of success or denied + * @throws RemoteException + */ + public int getOverrideWifiConfigPermission(int uid) throws RemoteException { + return AppGlobals.getPackageManager().checkUidPermission( + android.Manifest.permission.OVERRIDE_WIFI_CONFIG, uid); + } } diff --git a/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java b/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java index a00458726..5c78d0de3 100644 --- a/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java +++ b/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java @@ -44,6 +44,7 @@ import android.util.Pair; import com.android.internal.R; import com.android.server.wifi.WifiConfigStoreLegacy.WifiConfigStoreDataLegacy; +import com.android.server.wifi.util.WifiPermissionsUtil; import com.android.server.wifi.util.WifiPermissionsWrapper; import org.junit.After; @@ -95,7 +96,6 @@ public class WifiConfigManagerTest { private static final String TEST_PAC_PROXY_LOCATION_2 = "http://blah"; @Mock private Context mContext; - @Mock private FrameworkFacade mFrameworkFacade; @Mock private Clock mClock; @Mock private UserManager mUserManager; @Mock private TelephonyManager mTelephonyManager; @@ -104,6 +104,7 @@ public class WifiConfigManagerTest { @Mock private WifiConfigStoreLegacy mWifiConfigStoreLegacy; @Mock private PackageManager mPackageManager; @Mock private DevicePolicyManagerInternal mDevicePolicyManagerInternal; + @Mock private WifiPermissionsUtil mWifiPermissionsUtil; @Mock private WifiPermissionsWrapper mWifiPermissionsWrapper; @Mock private NetworkListStoreData mNetworkListStoreData; @Mock private DeletedEphemeralSsidsStoreData mDeletedEphemeralSsidsStoreData; @@ -163,17 +164,6 @@ public class WifiConfigManagerTest { } }).when(mPackageManager).getPackageUidAsUser(anyString(), anyInt(), anyInt()); - // Both the UID's in the test have the configuration override permission granted by - // default. This maybe modified for particular tests if needed. - doAnswer(new AnswerWithArguments() { - public int answer(String permName, int uid) throws Exception { - if (uid == TEST_CREATOR_UID || uid == TEST_UPDATE_UID || uid == TEST_SYSUI_UID) { - return PackageManager.PERMISSION_GRANTED; - } - return PackageManager.PERMISSION_DENIED; - } - }).when(mFrameworkFacade).checkUidPermission(anyString(), anyInt()); - when(mWifiKeyStore .updateNetworkKeys(any(WifiConfiguration.class), any(WifiConfiguration.class))) .thenReturn(true); @@ -182,6 +172,7 @@ public class WifiConfigManagerTest { when(mDevicePolicyManagerInternal.isActiveAdminWithPolicy(anyInt(), anyInt())) .thenReturn(false); + when(mWifiPermissionsUtil.checkConfigOverridePermission(anyInt())).thenReturn(true); when(mWifiPermissionsWrapper.getDevicePolicyManagerInternal()) .thenReturn(mDevicePolicyManagerInternal); createWifiConfigManager(); @@ -313,15 +304,7 @@ public class WifiConfigManagerTest { // Now change BSSID of the network. assertAndSetNetworkBSSID(openNetwork, TEST_BSSID); - // Deny permission for |UPDATE_UID|. - doAnswer(new AnswerWithArguments() { - public int answer(String permName, int uid) throws Exception { - if (uid == TEST_CREATOR_UID) { - return PackageManager.PERMISSION_GRANTED; - } - return PackageManager.PERMISSION_DENIED; - } - }).when(mFrameworkFacade).checkUidPermission(anyString(), anyInt()); + when(mWifiPermissionsUtil.checkConfigOverridePermission(anyInt())).thenReturn(false); // Update the same configuration and ensure that the operation failed. NetworkUpdateResult result = updateNetworkToWifiConfigManager(openNetwork); @@ -344,13 +327,6 @@ public class WifiConfigManagerTest { // Now change BSSID of the network. assertAndSetNetworkBSSID(openNetwork, TEST_BSSID); - // Deny permission for all UIDs. - doAnswer(new AnswerWithArguments() { - public int answer(String permName, int uid) throws Exception { - return PackageManager.PERMISSION_DENIED; - } - }).when(mFrameworkFacade).checkUidPermission(anyString(), anyInt()); - // Update the same configuration using the creator UID. NetworkUpdateResult result = mWifiConfigManager.addOrUpdateNetwork(openNetwork, TEST_CREATOR_UID); @@ -746,15 +722,7 @@ public class WifiConfigManagerTest { assertTrue(retrievedStatus.isNetworkEnabled()); verifyUpdateNetworkStatus(retrievedNetwork, WifiConfiguration.Status.ENABLED); - // Deny permission for |UPDATE_UID|. - doAnswer(new AnswerWithArguments() { - public int answer(String permName, int uid) throws Exception { - if (uid == TEST_CREATOR_UID) { - return PackageManager.PERMISSION_GRANTED; - } - return PackageManager.PERMISSION_DENIED; - } - }).when(mFrameworkFacade).checkUidPermission(anyString(), anyInt()); + when(mWifiPermissionsUtil.checkConfigOverridePermission(anyInt())).thenReturn(false); // Now try to set it disabled with |TEST_UPDATE_UID|, it should fail and the network // should remain enabled. @@ -783,15 +751,7 @@ public class WifiConfigManagerTest { mWifiConfigManager.getConfiguredNetwork(result.getNetworkId()); assertEquals(TEST_CREATOR_UID, retrievedNetwork.lastConnectUid); - // Deny permission for |UPDATE_UID|. - doAnswer(new AnswerWithArguments() { - public int answer(String permName, int uid) throws Exception { - if (uid == TEST_CREATOR_UID) { - return PackageManager.PERMISSION_GRANTED; - } - return PackageManager.PERMISSION_DENIED; - } - }).when(mFrameworkFacade).checkUidPermission(anyString(), anyInt()); + when(mWifiPermissionsUtil.checkConfigOverridePermission(anyInt())).thenReturn(false); // Now try to update the last connect UID with |TEST_UPDATE_UID|, it should fail and // the lastConnectUid should remain the same. @@ -3221,6 +3181,8 @@ public class WifiConfigManagerTest { when(mDevicePolicyManagerInternal.isActiveAdminWithPolicy(anyInt(), eq(DeviceAdminInfo.USES_POLICY_DEVICE_OWNER))) .thenReturn(withDeviceOwnerPolicy); + when(mWifiPermissionsUtil.checkConfigOverridePermission(anyInt())) + .thenReturn(withConfOverride); int uid = withConfOverride ? TEST_CREATOR_UID : TEST_NO_PERM_UID; NetworkUpdateResult result = mWifiConfigManager.addOrUpdateNetwork(network, uid); assertEquals(assertSuccess, result.getNetworkId() != WifiConfiguration.INVALID_NETWORK_ID); @@ -3230,9 +3192,9 @@ public class WifiConfigManagerTest { private void createWifiConfigManager() { mWifiConfigManager = new WifiConfigManager( - mContext, mFrameworkFacade, mClock, mUserManager, mTelephonyManager, + mContext, mClock, mUserManager, mTelephonyManager, mWifiKeyStore, mWifiConfigStore, mWifiConfigStoreLegacy, - mWifiPermissionsWrapper, mNetworkListStoreData, + mWifiPermissionsUtil, mWifiPermissionsWrapper, mNetworkListStoreData, mDeletedEphemeralSsidsStoreData); mWifiConfigManager.enableVerboseLogging(1); } diff --git a/tests/wifitests/src/com/android/server/wifi/WifiStateMachineTest.java b/tests/wifitests/src/com/android/server/wifi/WifiStateMachineTest.java index 9ab24c56b..a56773ec0 100644 --- a/tests/wifitests/src/com/android/server/wifi/WifiStateMachineTest.java +++ b/tests/wifitests/src/com/android/server/wifi/WifiStateMachineTest.java @@ -196,8 +196,6 @@ public class WifiStateMachineTest { } }); - when(facade.checkUidPermission(eq(android.Manifest.permission.OVERRIDE_WIFI_CONFIG), - anyInt())).thenReturn(PackageManager.PERMISSION_GRANTED); return facade; } diff --git a/tests/wifitests/src/com/android/server/wifi/util/WifiPermissionsUtilTest.java b/tests/wifitests/src/com/android/server/wifi/util/WifiPermissionsUtilTest.java index c783933cc..33495f904 100644 --- a/tests/wifitests/src/com/android/server/wifi/util/WifiPermissionsUtilTest.java +++ b/tests/wifitests/src/com/android/server/wifi/util/WifiPermissionsUtilTest.java @@ -17,6 +17,8 @@ package com.android.server.wifi.util; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.anyInt; import static org.mockito.Matchers.anyString; import static org.mockito.Mockito.doAnswer; @@ -32,13 +34,14 @@ import android.content.pm.PackageManager; import android.content.pm.UserInfo; import android.net.NetworkScoreManager; import android.os.Build; +import android.os.RemoteException; import android.os.UserHandle; import android.os.UserManager; import android.provider.Settings; import com.android.server.wifi.BinderUtil; +import com.android.server.wifi.FakeWifiLog; import com.android.server.wifi.WifiInjector; -import com.android.server.wifi.WifiLog; import com.android.server.wifi.WifiSettingsStore; import org.junit.Before; @@ -47,6 +50,7 @@ import org.junit.runner.RunWith; import org.junit.runners.JUnit4; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import org.mockito.Spy; import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; @@ -70,7 +74,7 @@ public class WifiPermissionsUtilTest { @Mock private ContentResolver mMockContentResolver; @Mock private NetworkScoreManager mNetworkScoreManager; @Mock private WifiInjector mWifiInjector; - @Mock private WifiLog mWifiLog; + @Spy private FakeWifiLog mWifiLog; private static final String TEST_PACKAGE_NAME = "com.google.somePackage"; private static final String INVALID_PACKAGE = "BAD_PACKAGE"; @@ -113,6 +117,51 @@ public class WifiPermissionsUtilTest { } /** + * Verify we return true when the UID does have the override config permission + */ + @Test + public void testCheckConfigOverridePermissionApproved() throws Exception { + mUid = MANAGED_PROFILE_UID; // do not really care about this value + setupTestCase(); + WifiPermissionsUtil codeUnderTest = new WifiPermissionsUtil(mMockPermissionsWrapper, + mMockContext, mMockWifiSettingsStore, mMockUserManager, mNetworkScoreManager, + mWifiInjector); + when(mMockPermissionsWrapper.getOverrideWifiConfigPermission(anyInt())) + .thenReturn(PackageManager.PERMISSION_GRANTED); + assertTrue(codeUnderTest.checkConfigOverridePermission(mUid)); + } + + /** + * Verify we return false when the UID does not have the override config permission. + */ + @Test + public void testCheckConfigOverridePermissionDenied() throws Exception { + mUid = OTHER_USER_UID; // do not really care about this value + setupTestCase(); + WifiPermissionsUtil codeUnderTest = new WifiPermissionsUtil(mMockPermissionsWrapper, + mMockContext, mMockWifiSettingsStore, mMockUserManager, mNetworkScoreManager, + mWifiInjector); + when(mMockPermissionsWrapper.getOverrideWifiConfigPermission(anyInt())) + .thenReturn(PackageManager.PERMISSION_DENIED); + assertFalse(codeUnderTest.checkConfigOverridePermission(mUid)); + } + + /** + * Verify we return false when the override config permission check throws a RemoteException. + */ + @Test + public void testCheckConfigOverridePermissionWithException() throws Exception { + mUid = OTHER_USER_UID; // do not really care about this value + setupTestCase(); + WifiPermissionsUtil codeUnderTest = new WifiPermissionsUtil(mMockPermissionsWrapper, + mMockContext, mMockWifiSettingsStore, mMockUserManager, mNetworkScoreManager, + mWifiInjector); + doThrow(new RemoteException("Failed to check permissions for " + mUid)) + .when(mMockPermissionsWrapper).getOverrideWifiConfigPermission(mUid); + assertFalse(codeUnderTest.checkConfigOverridePermission(mUid)); + } + + /** * Test case setting: Package is valid * Caller can read peers mac address * This App has permission to request WIFI_SCAN |